Re: [PATCH 1/3] command-list.txt: group common commands by theme
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- May 7, 2015, 16:50 UTC
- Message-ID
- <CAPig+cQ2e4c5hYsRbZhgyvLcMPsqshqRUZGNLwhJd57YP9JEFA@mail.gmail.com>
- In-Reply-To
- <554A8084.10506@gmail.com>
On Wed, May 6, 2015 at 4:58 PM, Sébastien Guimmara <sebastien.guimmara@gmail.com> wrote:
Show 20 quoted lines
> 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: >>> 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 ?
I'm not sure to what you are referring here? (Perhaps my comment was unclear, or perhaps I'm misreading your response.)
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".
Show 14 quoted lines
>>> +[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.
Show 11 quoted lines
>>> -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.)
Show 8 quoted lines
>>> -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.
Show 10 quoted lines
>>> [...] >>> -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