Re: [PATCH 2/3] generate-cmdlist.sh: parse common group commands

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

On 05/08/2015 05:20 AM, Eric Sunshine wrote:
> On Mon, May 04, 2015 at 10:28:09PM +0200, Sébastien Guimmara wrote:
>> parse the [groups] block to create the array of group descriptions
>>
>>     static char *common_cmd_groups[] = {
>>        N_("starting a working area"),
>>        ...
>>     };
>>
>> then map each element of common_cmds[] to a group via its index:
>>
>>     static struct cmdname_help common_cmds[] = {
>>       {"add", N_("Add file contents to the index"), 1},
>>       ...
>>
>> So that 'git help' can print those command grouped by theme.
>> ---
>> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
>> @@ -1,23 +1,42 @@
>> +content=$(cat command-list.txt)
>> +
>> +group_line_no=$(expr $(echo "$content" | grep -n '^\[groups\]' | cut -f1 -d:) + 1)
>> +command_line_no=$(expr $(echo "$content" | grep -n '^\[commands\]' | cut -f1 -d:) + 1)
>> +groups=$(echo "$content" | sed -n ''$group_line_no', '$(expr $command_line_no)'p')
>> [...]
>> +static char *common_cmd_groups[] = {"
>> +echo "$groups" |
>> +while read group description; do
>> +    if [ -z $group ]; then
>> +        break
>> +    fi
>> +    echo '   N_("'$description'"),'
>> +done
>> +echo "};
>> [...]
>> +echo "$content" | grep 'common-' |
>> +awk '{ print $1, "\t", $3 }' |
>> +while read cmd grp; do
>> +    cmd_name=$(echo $cmd | cut -d - -f 2)
>> +    group_name=$(echo $grp | cut -d - -f 2)
>> +    group_idx=$(expr $(echo "$groups" | grep -n "^$group_name" | cut -c 1) - 1)
>> +    sed -n '
>> +    /^NAME/,/git-'"$cmd_name"'/H
>> +    ${
>> +       x
>> +       s/.*git-'"$cmd_name"' - \(.*\)/  {"'"$cmd_name"'", N_("\1"), '"$group_idx"'},/
>> +       p
>> +    }' "Documentation/$cmd.txt"
>
> Background: In an earlier review, I observed[1] that the "common-" in
> the "common-N_group" form was redundant, and I suggested that you
> could add a [groups] section listing the groups, and that the order
> of items in [groups] would imply the "git help" display order of the
> groups, thus allowing you to do away with the "N_" qualifier, as
> well. I also observed that you could determine if a command in
> [commands] was common by checking if it was tagged with an attribute
> from [groups], thus alleviating the need for the "common-" prefix.
>
> This round makes nice headway toward the proposed scheme, although it
> still depends upon the redundant "common-" prefix. When I earlier
> suggested that awk could be helpful[2], I was thinking of its
> associative arrays which could be used to determine if a command in
> [commands] was tagged with an attribute from [groups].
>
> I had intended to reply to the current patch with a short "here's
> what I had in mind" example of using awk to achieve this goal,
> however, the short example ended up implementing the full
> functionality, so I went ahead and turned it into a proper patch[6]
> (below), and shamelessly re-used your commit message (with minor
> changes). You're welcome to include this patch in your re-roll, or
> use it as inspiration if you want to write the functionality
> yourself.
>
> Some notes about the re-implementation in awk: It assumes that
> [groups] has been renamed to [common] as suggested[3], and assumes
> that the "common-" prefix has been dropped from the [commands]
> attribute entries. The awk script replaces the current shell script
> entirely, and all common-cmds.h generation functionality is now
> handled by the one awk invocation rather than by a series of commands
> invoked by the shell script, which should make it faster (especially
> on Windows). Finally, unlike the shell script, the awk script does
> not bother sorting commands from command-list.txt since it assumes
> that command sorting will happen in parallel with grouping[4].
>
> When the awk script encounters [common], it begins collecting group
> names in a grp[] array and emits the appropriate common_cmd_groups[]
> "C" initializer for each. Upon encountering [commands], it switches
> mode and, for each command line, checks if any attribute with which a
> command is tagged exists in grp[]. If so, it emits the appropriate
> common_cmds[] "C" initializer. Comment and blank lines are skipped.
>
> By the way, Junio observed[5] that you will need to adjust a couple
> Makefiles (and such) to account for the new [common] section and
> [commands] header. A good start would be to filter command-list.txt
> via this command:
>
>      sed '1,/\[commands\]/d' <command-list.txt
>
> which will strip out everything up to and including the [commands]
> header.
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/268291
> [2]: http://article.gmane.org/gmane.comp.version-control.git/268294
> [3]: http://article.gmane.org/gmane.comp.version-control.git/268453
> [4]: http://article.gmane.org/gmane.comp.version-control.git/268442
> [5]: http://article.gmane.org/gmane.comp.version-control.git/268443
> [6]: Below is full patch which replaces 2/3 from this round:
>
> --- >8 ---
> From: Eric Sunshine <sunshine@sunshineco.com>
> Subject: [PATCH] generate-cmdlist: parse common group commands
>
> Parse the [common] block to create the array of group descriptions:
>
> static char *common_cmd_groups[] = {
>      N_("starting a working area"),
>      N_("working on the current change"),
>      N_("working with others"),
>      N_("examining the history and state"),
>      N_("growing, marking and tweaking your history"),
> };
>
> then map each element of common_cmds[] to a group via its index:
>
> static struct cmdname_help common_cmds[] = {
>      {"add", N_("Add file contents to the index"), 1},
>      {"branch", N_("List, create, or delete branches"), 4},
>      {"checkout", N_("Checkout a branch or paths to the ..."), 4},
>      {"clone", N_("Clone a repository into a new directory"), 0},
>      {"commit", N_("Record changes to the repository"), 4},
>      ...
> };
>
> so that 'git help' can print those commands grouped by theme.
>
> Only commands tagged with an attribute from [common] are emitted to
> common_cmds[].
>
> [commit message by Sébastien Guimmara <sebastien.guimmara@gmail.com>]
>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>   Makefile             |  4 ++--
>   generate-cmdlist.awk | 39 +++++++++++++++++++++++++++++++++++++++
>   generate-cmdlist.sh  | 23 -----------------------
>   3 files changed, 41 insertions(+), 25 deletions(-)
>   create mode 100644 generate-cmdlist.awk
>   delete mode 100755 generate-cmdlist.sh
>
> diff --git a/Makefile b/Makefile
> index 5f3987f..de28ae1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1687,10 +1687,10 @@ $(BUILT_INS): git$X
>   	ln -s $< $@ 2>/dev/null || \
>   	cp $< $@
>
> -common-cmds.h: ./generate-cmdlist.sh command-list.txt
> +common-cmds.h: generate-cmdlist.awk command-list.txt
>
>   common-cmds.h: $(wildcard Documentation/git-*.txt)
> -	$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@
> +	$(QUIET_GEN)awk -f generate-cmdlist.awk command-list.txt > $@+ && mv $@+ $@
>
>   SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
>   	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
> diff --git a/generate-cmdlist.awk b/generate-cmdlist.awk
> new file mode 100644
> index 0000000..19b36e5
> --- /dev/null
> +++ b/generate-cmdlist.awk
> @@ -0,0 +1,39 @@
> +BEGIN {
> +	print "/* Automatically generated */\n"
> +	print "struct cmdname_help {"
> +	print "\tchar name[16];"
> +	print "\tchar help[80];"
> +	print "\tunsigned char group;"
> +	print "};\n"
> +	print "static char *common_cmd_groups[] = {"
> +}
> +/^#/ || /^[ 	]*$/ { next }
> +state == 2 {
> +	for (i = 2; i <= NF; i++)
> +		if (grp[$i]) {
> +			f = "Documentation/"$1".txt"
> +			while (getline s <f > 0)
> +				if (match(s, $1" - ")) {
> +					t = substr(s, length($1" - ") + 1)
> +					break
> +				}
> +			close(f)
> +			printf "\t{\"%s\", N_(\"%s\"), %s},\n",
> +				substr($1, length("git-") + 1), t, grp[$i] - 1
> +			break
> +		}
> +}
> +/\[commands\]/ {
> +	print "};\n\nstatic struct cmdname_help common_cmds[] = {"
> +	state = 2
> +}
> +state == 1 {
> +	grp[$1] = ++n
> +	sub($1"[ 	][ 	]*", "")
> +	printf "\tN_(\"%s\"),\n", $0
> +	next
> +}
> +/\[common\]/ {
> +	state = 1
> +}
> +END { print "};" }
> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> deleted file mode 100755
> index 9a4c9b9..0000000
> --- a/generate-cmdlist.sh
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -#!/bin/sh
> -
> -echo "/* Automatically generated by $0 */
> -struct cmdname_help {
> -    char name[16];
> -    char help[80];
> -};
> -
> -static struct cmdname_help common_cmds[] = {"
> -
> -sed -n -e 's/^git-\([^ 	]*\)[ 	].* common.*/\1/p' command-list.txt |
> -sort |
> -while read cmd
> -do
> -     sed -n '
> -     /^NAME/,/git-'"$cmd"'/H
> -     ${
> -	    x
> -	    s/.*git-'"$cmd"' - \(.*\)/  {"'"$cmd"'", N_("\1")},/
> -	    p
> -     }' "Documentation/git-$cmd.txt"
> -done
> -echo "};"
>

Wow. Thanks very much. It has been added in the new version of the patch,
including the removal of the 'next' line.