Re: [PATCH v3 2/4] generate-cmdlist.sh: parse common command groups
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- May 3, 2015, 19:18 UTC
- Message-ID
- <CAPig+cQNOEgZAn=2MXNjkUxnMA+OimTLfSmEMskZsvRv-Zh42w@mail.gmail.com>
- In-Reply-To
- <55456A40.80806@gmail.com>
On Sat, May 2, 2015 at 8:22 PM, Sébastien Guimmara <sebastien.guimmara@gmail.com> wrote:
Show 28 quoted lines
> Teach generate-cmdlist.sh to parse common command groups
> found in command-list.txt in the form
>
> common-3_worktree ('3_worktree' being the group identifier)
>
> Extract the $grp variable, in addition to the previous $cmd,
> and inject it as a third field in the cmdname_help struct:
>
> {"add", N_("Add file contents to the index"), "3_worktree"},
>
> So that when 'git' is called, we can display common commands
> grouped by theme instead of a less useful alphabetical order.
>
> Signed-off-by: Sébastien Guimmara <sebastien.guimmara@gmail.com>
> ---
> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> index 9a4c9b9..98f937b 100755
> --- a/generate-cmdlist.sh
> +++ b/generate-cmdlist.sh
> @@ -4,19 +4,20 @@ echo "/* Automatically generated by $0 */
> struct cmdname_help {
> char name[16];
> char help[80];
> + char group[20];
> };
> static struct cmdname_help common_cmds[] = {"
> -sed -n -e 's/^git-\([^ ]*\)[ ].* common.*/\1/p' [...]
> +sed -n -e 's/^git-\([^ ]*\)[ ].* common-\(.*\)/\1:\2/p' [...]Isn't \(.*\) a bit too loose for grabbing $grp? What if someone some day adds another column to express some other sort of attribute? (Or, if someone perhaps uses the "deprecated" attribute but places it in column 3 rather than 2 as documented.) It probably would be better to tighten this up by grabbing only non-whitespace characters, as is already done for $cmd.
(In fact, this sed invocation is already broken for "deprecated", isn't it? According to the command-list.txt header comments, "deprecated" should be placed before "common", yet the sed invocation expects "common" to follow the command immediately. Perhaps a preparatory patch could fix this issue; or just do away with "deprecated" entirely since it's not used.)
> sort | > -while read cmd > +while IFS=: read cmd grp
Since both $cmd and $grp are simple tokens, neither of which contain whitespace, this could be handled more simply by just having sed emit them separated by a space rather than by ':'; in which case your 'while read' loop wouldn't need to muck with IFS at all.
Show 12 quoted lines
> do
> sed -n '
> /^NAME/,/git-'"$cmd"'/H
> ${
> x
> - s/.*git-'"$cmd"' - \(.*\)/ {"'"$cmd"'", N_("\1")},/
> + s/.*git-'"$cmd"' - \(.*\)/ {"'"$cmd"'", N_("\1"), "'"$grp"'"},/
> p
> }' "Documentation/git-$cmd.txt"
> done
> --
> 2.4.0