patchlaunch_editor(): allow spaces in the filename

For some reason, the construct

	sh -c "$0 \"$@\"" <editor> <file>

does not pick up quotes in <editor> as expected. So replace $0 with <editor>.

This fixes

	git config core.editor "c:/Program Files/What/Ever.exe"

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Incidentally, this fixes issue 78 in msysGit.

	Zeng (is this the correct way to address you?), Evan, please test.

 builtin-tag.c     |    6 +++++-
 t/t7005-editor.sh |   28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletions(-)

On Monday 10 March 2008 21:42, Johannes Schindelin wrote:

No surprise. It must be

	sh -c '"$0" "$@"' <editor> <file>

Note the extra quotes around $0.

IOW:

+			args[i++] = "\"$0\" \"$@\"";

-- Hannes

Johannes Sixt <johannes.sixt@telecom.at> writes:

... assuming that there is no dq, $var_reference and other nastiness in <editor> ;-)

On Monday 10 March 2008 22:37, Junio C Hamano wrote:

They are not a problem; there isn't yet another level of evaluation. I just tried with an <editor> that goes by this name:

   f "$DISPLAY"b

and I got the expected result, i.e. it was found and executed.

-- Hannes

Hi,

On Mon, 10 Mar 2008, Johannes Sixt wrote:

Isn't this wrong? What would this do with a core.editor like this:

[core]
	editor = this-editor --with-this --option

Hmm?

Ciao, Dscho

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

That's not "for some reason" but "I am asking shell to split", and

	sh -c '"$@"' ignoreme <editor> <files>...

would work better ;-).

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Having sent a few messages about shell quoting, it makes me wonder if this was done very deliberately in the first place, so that you can do things like:

	git config core.editor "emacs -nw"

Blame followed by list archive hunting around that timeperiod would tell.

Junio C Hamano <gitster@pobox.com> writes:

I'll let others look for the bug report and user request from the list archive, but the pertinent commit is 5e2de4f9 (Fix $EDITOR regression introduced by rewrite in C.):

    When git-tag and git-commit launches the editor, they used to
    honor EDITOR="editor -options args..." but recent rewrite in C
    insisted on $EDITOR to be the path to the editor executable.
    
    This restores the older behaviour.
    
In other words, your example is a user error and your patch is a
regression.

I guess the right way to do that would be:

	git config core.editor '"c:/Program Files/What/Ever.exe"'

but I do not do windows, so this is obviously untested.

Hi,

On Mon, 10 Mar 2008, Junio C Hamano wrote:

I was well aware of that commit when I wrote the patch, but...

Ooops. The commit message I gave did not escape the double quotes, but I _meant_ to, promise! So yes, it was my intention that

	git config core.editor \"C:/Program\ Files/vim/vim71/gvim.exe\"

works, but

	git config core.editor "C:/Program Files/vim/vim71/gvim.exe"

does not.  Note that the test was written correctly.

Sorry, Dscho

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Ah, I misread your patch. The above example needs to be fixed as discussed, but what the patch does to builtin-tag.c is fine.

Why do you need --exec-path=. to tell the newly built git to find its subcommands from t/trash directory, when you give an explicit instruction to find GIT_EDITOR in "./"?

For some reason, the construct

	sh -c "$0 \"$@\"" <editor> <file>

does not pick up quotes in <editor> as expected. So replace $0 with <editor>.

This fixes

	git config core.editor '"c:/Program Files/What/Ever.exe"'

In other words, you can specify an editor with spaces in its path using a config containing something like this:

	[core]
		editor = \"c:/Program Files/Darn/Spaces.exe\"

NOTE: we cannot just replace the $0 with \"$0\", because we still want 
this to work:

	[core]
		editor = emacs -nw

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Mon, 10 Mar 2008, Junio C Hamano wrote:

	> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
	> 
	> > For some reason, the construct
	> >
	> > 	sh -c "$0 \"$@\"" <editor> <file>
	> >
	> > does not pick up quotes in <editor> as expected.  So replace 
	> > $0 with <editor>.
	> >
	> > This fixes
	> >
	> > 	git config core.editor "c:/Program Files/What/Ever.exe"
	> 
	> Ah, I misread your patch.  The above example needs to be fixed 
	> as discussed, but what the patch does to builtin-tag.c is fine.

	Here comes the fixed commit message and...

	> > diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
	> > index c1cec55..5395b74 100755
	> > --- a/t/t7005-editor.sh
	> > +++ b/t/t7005-editor.sh
	> > @@ -89,6 +89,34 @@ do
	> >  	'
	> >  done
	> >  
	> > +test_expect_success 'editor with a space' '
	> > +
	> > +	if echo "echo space > \"\$1\"" > "e space.sh"
	> > +	then
	> > +		chmod a+x "e space.sh" &&
	> > +		GIT_EDITOR="./e\ space.sh" \
	> > +			git --exec-path=. commit --amend &&
	> 
	> Why do you need --exec-path=. to tell the newly built git to 
	> find its subcommands from t/trash directory, when you give an explicit 
	> instruction to find GIT_EDITOR in "./"?

	... the fixed tests.

	I copied this piece from the earlier tests.  Probably it is only 
	needed to avoid "vi" from being found in the PATH (--exec-path's argument 
	is prepended to the PATH).

 builtin-tag.c     |    6 +++++-
 t/t7005-editor.sh |   27 +++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletions(-)