Luke Diamand <luke@xxxxxxxxxxx> writes: > On 07/05/15 22:16, Junio C Hamano wrote: >> Junio C Hamano <gitster@xxxxxxxxx> writes: >> >> i.e. the shell at the beginning of system sees "$customEditor" >> (including the double quotes) as a quoted variable, expand the >> environment variable as exported, and treat it as the path to >> the program. Again untested but I think >> >> P4EDITOR="\"\$customEditor\"" && > > Or will this work? > > - P4EDITOR="$TRASH_DIRECTORY/ed.sh" && > + P4EDITOR="\"$TRASH_DIRECTORY/ed.sh\"" && I wasn't sure TRASH_DIRECTORY was exported; as long as it is (and it seems to be, from lib-test-functions.sh), that should be sufficient. > I'm still a bit worried about what will happen to Windows users with > this change though. I think the following avoids breaking Windows > clients,... > > - system([editor, template_file]) > + system(shlex.split(editor) + [template_file]) > > I've not tested it on anything other than Linux so far, so best not to > merge yet! Looking at run-command.c, GIT_WINDOES_NATIVE and POSIX seems to use pretty much the same construct, except that they use SHELL_PATH instead of "sh". So something like this may be sufficient, perhaps? Makefile | 1 + git-p4.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 20058f1..fda44bf 100644 --- a/Makefile +++ b/Makefile @@ -1776,6 +1776,7 @@ $(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS $(SCRIPT_PYTHON_GEN): % : %.py $(QUIET_GEN)$(RM) $@ $@+ && \ sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \ + -e 's|SHELL_PATH|$(SHELL_PATH_SQ)|g' \ $< >$@+ && \ chmod +x $@+ && \ mv $@+ $@ diff --git a/git-p4.py b/git-p4.py index de06046..eb6d4b1 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1220,7 +1220,7 @@ class P4Submit(Command, P4UserMap): editor = os.environ.get("P4EDITOR") else: editor = read_pipe("git var GIT_EDITOR").strip() - system(["sh", "-c", ('%s "$@"' % editor), editor, template_file]) + system(['''SHELL_PATH''', "-c", ('%s "$@"' % editor), editor, template_file]) # If the file was not saved, prompt to see if this patch should # be skipped. But skip this verification step if configured so. -- 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