[darcs-users] darcs patch: Refactor 'darcs --commands' to list_avai... (and 8 more)

Dmitry Kurochkin dmitry.kurochkin at gmail.com
Thu Oct 16 14:59:54 UTC 2008


On Thu, Oct 16, 2008 at 6:41 PM, David Roundy <droundy at darcs.net> wrote:
> On Wed, Oct 15, 2008 at 11:18:26AM -0700, Dmitry Kurochkin wrote:
>> Hello!
>>
>> This is mostly refactor of Darcs.Commands module proposed by David in
>> http://lists.osuosl.org/pipermail/darcs-users/2008-October/014214.html .
>>
>> Regards,
>>   Dmitry
>>
>> Wed Oct 15 16:58:27 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Refactor 'darcs --commands' to list_available_commands.
>
> Looks good.
>
>> Wed Oct 15 17:29:42 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Use help_cmd directly in main.
>
> Simple enough.
>
>> Wed Oct 15 17:42:36 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Use help_cmd instead of usage in main.
>
> This one changes behavior, causing darcs to exit with success when
> given no arguments.  I think that's fine, but it is a change.

Yes, I know that. But I think it is harmless enough.

>
>> Wed Oct 15 18:25:03 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Remove unused extended_usage.
>
> This is odd.  It's been dead code for some time, so far as I can
> see... nice to clear this out.
>
>> Wed Oct 15 21:40:31 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Refactor Darcs.Commands, move command run code to Darcs.RunCommand.
>
> Looks good.  I haven't actually checked, but I presume you made no
> changes to functionality here?

There are no functional changes unless I made a mistake.

>
>> Wed Oct 15 21:55:40 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Refactor Darcs.ArgumentDefaults to use command_control_list directly.
>
> Looks good.  This was the change that motivated this refactor, right?

Yes.

>
>> Wed Oct 15 22:06:29 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Fix imports in Darcs.ArgumentDefaults.
>
> Thanks for exparating out formatting changes from other changes! And
> it'll be even better if you mention that it's a formatting changes in
> the patch name...

Will keep in mind for the future.

>
>> Wed Oct 15 22:09:40 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Print commands options in 'darcs --commands'.
>
> Hmmm.  I'm not convinced by this one.  The --commands is used
> (almost?) exclusively for command-line completion, and I don't see a
> reason for users to want to call it for any other reason.  Similarly
> darcs changes --list-options doesn't mention --list-options.  If
> you're calling either of these commands, you don't need darcs to
> explain to you that they exist--and keeping the number of completions
> down is good for our UI.

I did not know about --list-options.

Regards,
  Dmitry

>
>> Wed Oct 15 22:11:41 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin at gmail.com>
>>   * Use command_control_list directly in run_the_command.
>
> Looks good!
>
> So all but one are applied.  Thanks for the refactor!
>
> David
> _______________________________________________
> darcs-users mailing list
> darcs-users at darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-users
>


More information about the darcs-users mailing list