Re: [PATCH 2/7] help -a: do not list commands that are excluded from the build

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Mon, 15 Apr 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> >  SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
> >  	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
> > diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> > index 709d67405b..7867b99d19 100755
> > --- a/generate-cmdlist.sh
> > +++ b/generate-cmdlist.sh
> > @@ -6,7 +6,7 @@ die () {
> >  }
> >
> >  command_list () {
> > -	grep -v '^#' "$1"
> > +	eval grep -ve '^#' $exclude_programs "$1"
>
> The original protects against $IFS in the filename given as $1, but
> with the eval that is no longer true.  We have been feeding the
> constant "command-list.txt" to the program since its inception, and
> I do not expect it to change, so this regression in defensiveness
> does not matter in practice.  Also because # is prefixed with ^, the
> unintended loss of quotes around it when the eval makes the shell
> re-parse the generated command line would not make the remainder
> into a comment.
>
> But it does look brittle, and smells like a trap for less
> experienced shell programmers to blindly copy & paste & tweak
> without understanding what is going on, leading to bugs.
>
> 	eval "grep -v -e '^#' $exclude_programs" <"$1"
>
> or something like that, perhaps?

Yes! Thank you.

> > @@ -93,6 +93,14 @@ EOF
> >  EOF
> >  }
> >
> > +exclude_programs=
> > +while test "a$1" = "a--exclude-program"
>
> s/a/z/g; if you want to pretend to be old-fashioned, but s/a//g;
> should be sufficient in the modern world.

Don't you know, I always used "a" without realizing. You can see my
misdeed even in git-rebase--preserve-merges.sh. I won't fix it there,
though, as I hope that it'll be gone soon enough.

Will be fixed in the next iteration I send.

Thanks,
Dscho