From: Eric Sunshine Date: Sun, 03 May 2015 20:10:55 GMT Subject: Re: [PATCH v3 2/4] generate-cmdlist.sh: parse common command groups Message-ID: In-Reply-To: On Sun, May 3, 2015 at 3:18 PM, Eric Sunshine wrote: > On Sat, May 2, 2015 at 8:22 PM, Sébastien Guimmara > wrote: >> --- >> 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 */ >> }; >> 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.) Ah, I misread the original sed invocation slightly. It's not broken, so scratch the second paragraph. (Though, it does expect at least one whitespace character and then one space before "common", which is a bit odd, but not a problem with the current content of command-list.txt.)