Re: [PATCH v2 2/3] git help: group common commands by theme

From: Sébastien Guimmara <sebastien.guimmara@gmail.com>
Date: May 2, 2015, 11:09 UTC
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:

Show 17 quoted lines

>> 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.

Show 20 quoted lines

>>   };
>>
>>   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