Thread (2 messages) 2 messages, 2 authors, 2019-03-25

Re: [PATCH 1/1] check-docs: fix for setups where executables have an extension

From: Johannes Schindelin <hidden>
Date: 2019-03-25 21:08:59

Hi Junio,

On Sun, 24 Mar 2019, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help