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

From: Sébastien Guimmara <[email-masked]>
Date: Fri, 08 May 2015 20:43:21 +0200

On 05/07/2015 06:50 PM, Eric Sunshine wrote:
> On Wed, May 6, 2015 at 4:58 PM, Sébastien Guimmara
> <[email-masked]> wrote:
>> On 05/06/2015 08:57 AM, Eric Sunshine wrote:
>>> On Mon, May 4, 2015 at 4:28 PM, Sébastien Guimmara
>>> <[email-masked]> wrote:
>>>> Signed-off-by: Sébastien Guimmara <[email-masked]>
>>>> ---
>>>> 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 ?
>
> I'm not sure to what you are referring here? (Perhaps my comment was
> unclear, or perhaps I'm misreading your response.)
>

I was mistaken, I thought you wanted to reorder the commands
alphabetically, which was unnecessary since they were sorted in
help.c anyway. But yes, I shall add a comment in command-list.txt

> I meant only that the comment above [groups] should say that the order
> of the items in [groups] is the order in which the groups themselves
> are output by "git help".
>
>>>> +[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).
>
> I also think "collaborating" itself is best, but changed it at the
> last second before sending the email.
>
>>>> -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.
>
> Strange. In my editor, all columns are already aligned. Perhaps your
> tab with setting is incorrect? (It should be set to 8.)
>

Actually I only removed the few tabs that were wandering in some lines
to replace them by spaces (almost all lines were space aligned, only a few
were tab aligned).

>>>> -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.
>
> I personally consider "grep" an important beginner command, but that's
> an issue to be argued separately; and it's also why it's a good idea
> to put the removals in their own patch, so people can argue about it
> without holding up the rest of the patches.
>
>>>>    [...]
>>>> -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.
>
> Historically, many Unix tools incorrectly handled files lacking that
> final newline; sometimes by dropping the line altogether, sometimes
> mis-processing it in some way or another. Misbehaviors still exist
> today, often in BSD tools. In fact, just a few days ago, such a
> problem was reported for git-filter-branch[1]. Consequently, retaining
> newline is good insurance against misbehaving tools.
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/267828/focus=267957
>