# [PATCHv2 0/2] fix handling of multi-word P4EDITOR 4 messages from 2015-05-13 to 2015-05-14. Participants: Luke Diamand, Junio C Hamano. Thread: https://gitlist.dev/t/39320 ## Luke Diamand, 2015-05-13 07:36 Subject: [PATCHv2 0/2] fix handling of multi-word P4EDITOR Message-ID: <1431502575-2176-1-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1431502575-2176-1-git-send-email-luke%40diamand.org ``` This is an updated version of the earlier P4EDITOR multi-word shell expansion fix. It updates test 9805 to match the new behavior (the other tests also all pass). I still haven't incorporated Junio's fix for the Windows platform; that's next (once I've figured out how to build git on that platform). Thanks Luke Luke Diamand (2): git-p4: add failing test for P4EDITOR handling git-p4: fix handling of multi-word P4EDITOR git-p4.py | 2 +- t/t9805-git-p4-skip-submit-edit.sh | 2 +- t/t9820-git-p4-editor-handling.sh | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100755 t/t9820-git-p4-editor-handling.sh -- 2.4.0.rc3.380.g8e2ddc7 ``` ## Luke Diamand, 2015-05-13 07:36 Subject: [PATCHv2 1/2] git-p4: add failing test for P4EDITOR handling Message-ID: <1431502575-2176-2-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1431502575-2176-2-git-send-email-luke%40diamand.org In-Reply-To: <1431502575-2176-1-git-send-email-luke@diamand.org> ``` Add test case that git-p4 handles a setting of P4EDITOR that takes arguments, e.g. "gvim -f" Signed-off-by: Luke Diamand Signed-off-by: Junio C Hamano --- t/t9820-git-p4-editor-handling.sh | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100755 t/t9820-git-p4-editor-handling.sh diff --git a/t/t9820-git-p4-editor-handling.sh b/t/t9820-git-p4-editor-handling.sh new file mode 100755 index 0000000..e0a3c52 --- /dev/null +++ b/t/t9820-git-p4-editor-handling.sh @@ -0,0 +1,38 @@ +#!/bin/sh + +test_description='git p4 handling of EDITOR' + +. ./lib-git-p4.sh + +test_expect_success 'start p4d' ' + start_p4d +' + +test_expect_success 'init depot' ' + ( + cd "$cli" && + echo file1 >file1 && + p4 add file1 && + p4 submit -d "file1" + ) +' + +test_expect_failure 'EDITOR has options' ' +# Check that the P4EDITOR argument can be given command-line +# options, which git-p4 will then pass through to the shell. + git p4 clone --dest="$git" //depot && + test_when_finished cleanup_git && + ( + cd "$git" && + echo change >file1 && + git commit -m "change" file1 && + P4EDITOR="touch \"$git/touched\"" git p4 submit && + test_path_is_file "$git/touched" + ) +' + +test_expect_success 'kill p4d' ' + kill_p4d +' + +test_done -- 2.4.0.rc3.380.g8e2ddc7 ``` ## Luke Diamand, 2015-05-13 07:36 Subject: [PATCHv2 2/2] git-p4: fix handling of multi-word P4EDITOR Message-ID: <1431502575-2176-3-git-send-email-luke@diamand.org> URL: https://gitlist.dev/e/1431502575-2176-3-git-send-email-luke%40diamand.org In-Reply-To: <1431502575-2176-1-git-send-email-luke@diamand.org> ``` This teaches git-p4 to pass the P4EDITOR variable to the shell for expansion, so that any command-line arguments are correctly handled. Without this, git-p4 can only launch the editor if P4EDITOR is solely the path to the binary, without any arguments. This also fixes t9805, which relied on the previous behaviour. Suggested-by: Jonathan Nieder Signed-off-by: Luke Diamand Signed-off-by: Junio C Hamano --- git-p4.py | 2 +- t/t9805-git-p4-skip-submit-edit.sh | 2 +- t/t9820-git-p4-editor-handling.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index 41a77e6..ca6bb95 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1248,7 +1248,7 @@ class P4Submit(Command, P4UserMap): editor = os.environ.get("P4EDITOR") else: editor = read_pipe("git var GIT_EDITOR").strip() - system([editor, template_file]) + system(["sh", "-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. diff --git a/t/t9805-git-p4-skip-submit-edit.sh b/t/t9805-git-p4-skip-submit-edit.sh index 8931188..5fbf904 100755 --- a/t/t9805-git-p4-skip-submit-edit.sh +++ b/t/t9805-git-p4-skip-submit-edit.sh @@ -90,7 +90,7 @@ test_expect_success 'no config, edited' ' cd "$git" && echo line >>file1 && git commit -a -m "change 5" && - P4EDITOR="$TRASH_DIRECTORY/ed.sh" && + P4EDITOR="\"$TRASH_DIRECTORY/ed.sh\"" && export P4EDITOR && git p4 submit && p4 changes //depot/... >wc && diff --git a/t/t9820-git-p4-editor-handling.sh b/t/t9820-git-p4-editor-handling.sh index e0a3c52..c178bd7 100755 --- a/t/t9820-git-p4-editor-handling.sh +++ b/t/t9820-git-p4-editor-handling.sh @@ -17,9 +17,9 @@ test_expect_success 'init depot' ' ) ' -test_expect_failure 'EDITOR has options' ' # Check that the P4EDITOR argument can be given command-line # options, which git-p4 will then pass through to the shell. +test_expect_success 'EDITOR has options' ' git p4 clone --dest="$git" //depot && test_when_finished cleanup_git && ( -- 2.4.0.rc3.380.g8e2ddc7 ``` ## Junio C Hamano, 2015-05-14 19:09 Subject: Re: [PATCHv2 0/2] fix handling of multi-word P4EDITOR Message-ID: URL: https://gitlist.dev/e/xmqqzj57j7sc.fsf%40gitster.dls.corp.google.com In-Reply-To: <1431502575-2176-1-git-send-email-luke@diamand.org> ``` Thanks, will replace. But with a couple of minor tweaks. - s/touch/: >/ in the test added by 1/2 - s/fixes t9850/adjusts t9805/ in the log message of 2/2 ```