From: Sébastien Guimmara <sebastien.guimmara@gmail.com>
Date: Sat, 02 May 2015 11:09:13 GMT
Subject: Re: [PATCH v2 2/3] git help: group common commands by theme
Message-ID: <5544B059.20901@gmail.com>
In-Reply-To: <CAE5ih78xxHj-YZVnNB=kNw5ZOkT1t2dcc5yjiUWemEQpwFQfDw@mail.gmail.com>

Hello,

On 05/02/2015 08:32 AM, Luke Diamand wrote:
>> Signed-off by Sébastien Guimmara <sebastien.guimmara@gmail.com>
>> ---
>>   generate-cmdlist.sh | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>>
>> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
>> index 9a4c9b9..818b2f3 100755
>> --- a/generate-cmdlist.sh
>> +++ b/generate-cmdlist.sh
>> @@ -4,19 +4,22 @@ echo "/* Automatically generated by $0 */
>>   struct cmdname_help {
>>       char name[16];
>>       char help[80];
>> +    char group[20];
> Is 20 chars long enough? git-p4 shows up as "foreignscminterface"
> which is pretty close to that limit.

This 'foreignscminterface' is actually a category in the 2nd column
of the file (like 'porcelain'). It is unrelated to common command
groups (3rd column). Since those group names are not meant to be printed,
I guess it is sufficient to keep them short.
>>   };
>>
>>   static struct cmdname_help common_cmds[] = {"
>>
>> -sed -n -e 's/^git-\([^     ]*\)[     ].* common.*/\1/p' command-list.txt |
>> +sed -n -e 's/^git-\([^     ]*\)[     ].* common-\(.*\)/\1:\2/p'
>> command-list.txt |
>>   sort |
>> -while read cmd
>> +while read line
>>   do
>> +     cmd=`echo $line | cut -d ':' -f 1`
>> +     grp=`echo $line | cut -d ':' -f 2`
> Should this use $(...) rather than `...` ?
> i.e.
> cmd=$(echo $(line | cut -d : -f 1)
>
> (And I don't think you need the quotes around the ':'.
>
> Luke
Indeed, the quotes are unnecessary and $(...) seems to be more
appropriate too.
I'll update the patch.

Thank you,

Sébastien