Re: [PATCH 1/3] command-list.txt: group common commands by theme

["Sébastien Guimmara" <sebastien.guimmara@gmail.com>]

On 05/06/2015 08:57 AM, Eric Sunshine wrote:
> On Mon, May 4, 2015 at 4:28 PM, Sébastien Guimmara
> <sebastien.guimmara@gmail.com> wrote:
>> Declare groups for common commands in the [groups] block,
>> followed by group names and descriptions:
>>
>>     [groups]
>>     init                   starting a working area
>>     worktree               working on the current change
>>     remote                 working with others
>>     info                   examining the history and state
>>     history                growing, marking and tweaking your history
>>
>> Then, in the [commands] block, map all common commands with a group:
>>
>>     [commands]
>>     git-add        mainporcelain     common-worktree
>>     git-branch     mainporcelain     common-history
>>     git-checkout   mainporcelain     common-history
>>     [...]
>>
>> command names and groups are then parsed with generate-cmdlist.sh to
>> generate common-commands.h.
>>
>> Those commands are displayed in groups in the output of 'git help'.
>
> It probably also is important to mention that the order of the items
> in [groups] is the order in which groups are output by 'git help'

Yes. I'll add a comment in the file as well as in the commit.

>
> More below.
>
>> Signed-off-by: Sébastien Guimmara <sebastien.guimmara@gmail.com>
>> ---
>> diff --git a/command-list.txt b/command-list.txt
>> index f1eae08..64394ca 100644
>> --- a/command-list.txt
>> +++ b/command-list.txt
>> @@ -1,29 +1,39 @@
>>   # List of known git commands.
>> -# command name                         category [deprecated] [common]
>> -git-add                                 mainporcelain common
>> +# only add group information for common commands
>
> Perhaps mention also that the order of groups here is the order in
> which they are output by 'git help'?

It wouldn't be necessary if we reorder alphabetically the content of
each group, no ?

>
>> +[groups]
>
> Thinking on this a bit more, perhaps [groups] is too generic. Maybe
> [common] or [commongroups] would be more descriptive?
>
>> +init                   starting a working area
>> +worktree               working on the current change
>> +remote                 working with others
>
> "collaborating with others" perhaps?

Yes, "groups" has been itching a bit. I thought about "theme", but
common just does the job too. "collaborating with others" sounds
redundant to me (but I'm being a grammar nazi here).

>> -git-fast-export                                ancillarymanipulators
>> -git-fast-import                                ancillarymanipulators
>> -git-fetch                               mainporcelain common
>> +git-fast-export                         ancillarymanipulators
>> +git-fast-import                         ancillarymanipulators
>
> Unintended whitespace changes for fast-export and fast-import lines? I
> wouldn't have expected to see these lines change in this patch.
>

All whitespace changes were intended to align the commands on the same
column. I realize this should be the object of a separate patch.

>> -git-grep                                mainporcelain common
>> +git-grep                                mainporcelain
>
> This change isn't mentioned anywhere, not even in the cover letter.
> Did you intend to drop 'grep' from the common command list?

It's a mistake in the cover letter. I indeed intended to propose to
remove grep and tag from the common commands.

>>   [...]
>> -git-write-tree                          plumbingmanipulators
>> +git-write-tree                          plumbingmanipulators
>> \ No newline at end of file
>
> Your editor is perhaps dropping the final newline in the file? This is
> an undesirable change. Patch 2/3 exhibits the same problem.

As for the final newline, it was deliberately removed. I was not aware it
was necessary in text files. I'll correct this.

Thank you for the help,

Sébastien