Re: [Monotone-devel] Help on subcommands (help-rewrite branch)

[Markus Schiltknecht]

Hi,

Julio M. Merino Vidal wrote:
> I think that the n.v.m.help-rewrite branch is in pretty good shape now.  
> Basically, it has the following changes over n.v.m:

cool. Two questions popped up when reviewing your changes compared to 
net.venge.monotone:

I've stumbled across the following comment in cmd.hh:

     // NB: these strings are stored _un_translated, because we cannot
     // translate them until after main starts, by which time the
     // command objects have all been constructed.

Does your change from 'std::string name' to 'utf8 m_primary_name' affect 
that in any way? Do you know what that comment means?


Why is command a struct, but automate a class? Why does automate have an 
exec() and a run() method? Is it possible to merge that into one?

> - The CMD* macros now take a parameter to the parent command (not a
>   string mentioning the group they are in).  This allows the
>   construction of a tree of commands rather than a plain list.
> - The CMD* macros also take an 'abstract' parameter to specify a
>   brief description for each command.  This is shown in command
>   summaries.
> - CMD_WITH_SUBCMDS is gone.  Commands that take subcommands are now
>   defined with the more generic CMD_GROUP.

Oh, I like that one!

> - Automation commands are defined with CMD_AUTOMATE and are added to
>   the same tree of commands.

And that's great, too.

> With all the above internal changes, 'mtn help' is now able to lookup 
> help for commands at any level.

Cool!

> Could you please review the code and tell me if it is appropriate for 
> mainline?  If so, I would like to merge it after 0.35 has been released 
> (and after the above pending things have been addressed).

I'm looking forward to merging your changes into my 
experiment.encapsulation branch as soon as possible.

> There are other changes I'd like to try that involve removing the 
> special-casing of top-level commands (see the comment at the beginning 
> of commands.cc).  But these may result in some command-line 
> incompatibilities, and I'd rather not do these on a first attempt.  
> I.e., I'd first like to get the refactoring merged into mainline without 
> losing compatibility and, later on, propose these new ideas.

Sounds good from here.

[ BTW: your branch compiled and passed the testsuite successfully on my 
(quite standard AMD Debian GNU/Linux) machine here at work. ]

Regards

Markus