Hi Junio, On Sun, 24 Mar 2019, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schinde...@gmx.de> 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. You are right, I should have root-caused it, it had a funny smell to it. Turns out that we should not just skip those lines with a single token, but instead fix the bug that prevents the `how` variable to be set to `documented` in those cases, as had been intended all along. I fixed the commit, and accompanied it with a bug fix for this, plus for the fall-out bugs that had been hidden by this bug. Ciao, Dscho
