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

David Roundy droundy at darcs.net
Thu Oct 16 14:41:41 UTC 2008


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.

> 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?

> 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?

> 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...

> 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.

> 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


More information about the darcs-users mailing list