Re: [PATCH] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')

[Jeff King <peff@peff.net>]

On Wed, Nov 20, 2013 at 09:22:31AM -0800, Junio C Hamano wrote:

> > commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
> > from the commit message", 19-11-2013) in 'pu' fails the new test
> > it added to t7507.
> >
> > I didn't spend too long looking at the problem, so take this patch
> > as nothing more than a quick suggestion for a possible solution! :-P
> > [The err file contained something like: "There was a problem with the
> > editor '"$FAKE_EDITOR"'"].
> >
> > Having said that, this fixes it for me ...
> 
> Well spotted.  test_must_fail being a shell function, not a command,
> we shouldn't have used the "VAR=val cmd" one-shot environment
> assignment for portability.

Yeah, I noticed that, too upon reading Ramsay's patch. But I thought the
usual symptom there was that the variable is not properly unset after
the function returns? In other words, it might affect later tests, but
the failure that Ramsay sees is in _this_ test, so it must be a separate
issue.

The test_set_editor helper does some magic to help with quoting, but
that should not be an issue in this case (since we are using "cat"). We
are using test_set_editor elsewhere in the script, which would have set
EDITOR previously. But I would think that GIT_EDITOR, which we are using
here, would supersede that. However, the error message he shows
indicates that git is using EDITOR (as FAKE_EDITOR is part of that quote
magic).

Am I misremembering the issues with one-shot variables and functions?

Puzzled...

-Peff