Junio C Hamano <gitster@xxxxxxxxx> writes:
> David Kastrup <dak@xxxxxxx> writes:
>
>> Actually, splicing $EDITOR into a system command is a nuisance because
>> it means having to shell-quote its arguments. So the current
>> interpretation is likely easier to maintain.
>>
>> Is it the correct one?
>
> I've been torn on this one. From the point of view of
> "specified behaviour in the documentation", which is "EDITOR and
> VISUAL name the editor of your choice", not splicing is not
> violating the letter (I am not talking about our documentation
> here, but many other programs'). Splicing and shell quoting
> other parameters, while it is technically not a problem at all
> doing that in scripts, feels "dirty". Maybe it's just me.
>
> Both cvs and svn seems to splice, I suspect they just do a
> straight system(3) invocation.
>
> We recently normalized the script callers not to splice at all
> (the scripts were hand-rolling "the VISUAL or EDITOR or vi" and
> slightly differently). It obviously has negative (i.e. setting
> EDITOR to "emacsclient --alternate-editor vi" does not work) as
> well as positive side (i.e. "/home/dak/My Programs/editor" would
> work).
Well, I just checked the behavior with "less", "more", "mail" and
"mailx", quite traditional commands that would not have a reason to
complicate things by using "system" and quoting instead of exec.
less, mail and mailx apparently go via system, more (wtf?!?)
apparently via exec.
Taken together with the behavior by cvs and svn, I think we should not
just throw EDITOR/VISUAL into one exec arg.
Then there are two implementations to pick from:
a) Using system and shell-quoting the filename. Advantage: one can
set EDITOR='"/home/dak/My Programs/editor"' and have it work.
Disadvantage: shell-quoting a file name seems shell- and
system-dependent.
It turns out all three contestants still in the race apparently do a).
If nobody has a sensible idea how to shell-quote generally enough...
Under Unix, one has the option of using "..." and quoting the three of
"$\ with \ or using '...' and replacing every contained ' with '\''.
I don't think that there is a library function generally available.
The " quote type would probably be more typical.
It turns out that gitk and gitk-wish _already_ do this. So the
normalization does not seem to have covered them if I read the code
correctly:
proc shellquote {str} {
if {![string match "*\['\"\\ \t]*" $str]} {
return $str
}
if {![string match "*\['\"\\]*" $str]} {
return "\"$str\""
}
if {![string match "*'*" $str]} {
return "'$str'"
}
return "\"[string map {\" \\\" \\ \\\\} $str]\""
}
Note that the first case does not cover strings with newlines in them,
though, and the second does not help with dollar signs. And I have no
clue what the last return does. Presumably maps " to \" and \ to \\
inside of double quotes.
b) splitting EDITOR/VISUAL at spaces and using exec. Nobody else
appears to do this, so may neither should be.
It appears that the C code already has quote.c, so it is probably more
or less doable.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
