Johannes Schindelin <[email protected]> writes:
> Sharp eyes, and a *very* good point. The double space is actually required
> for this patch to work as intended. I added the following explanation to
> the commit message:
>
> Note that `$(ALL_COMMANDS)` starts with a space, and that is rather
> crucial for the `while read how cmd` loop: some of the input lines do
> not actually have the form `<how> <cmd>`, but only `<cmd>`, therefore
> `$cmd` evaluates to the empty string, and when we are looking for
> `* $cmd *` in ` $(ALL_COMMANDS)`, we do find it because the latter
> starts with a double space.
> In other words, the double space helps us skip the lines that list
> only a command.
That sounds more like a bug in the existing set-up even before your
patch (in fact, these lines are probably from 2007 or so, long
before you started touching our top-level Makefile heavily). If we
want to ignore lines with only one token, I'd rather see it
explicitly done, e.g.
... generate data ... |
while read how cmd
do
# skip a line with a single token
case "$cmd" in "") continue ;; esac
# is $cmd part of the known command list?
case " $(ALL_COMMANDS) " in
*" $cmd "*) ;; # skip
*) echo ... warning ... ;;
esac
done
instead of relying on "if $cmd is empty, then SP + $cmd + SP makes
double space, so let's have double space somewhere in the list of
known commands" cuteness.
Thanks.
