{"thread":{"id":"39320","subject":"[PATCHv2 0/2] fix handling of multi-word P4EDITOR","startedAt":"2015-05-13T07:36:13Z","lastAt":"2015-05-14T19:09:07Z","messageCount":4,"participants":["Luke Diamand","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"261136","messageId":"1431502575-2176-1-git-send-email-luke@diamand.org","threadId":"39320","inReplyTo":null,"subject":"[PATCHv2 0/2] fix handling of multi-word P4EDITOR","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-05-13T07:36:13Z","receivedAt":"2015-05-13T07:36:13Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"This is an updated version of the earlier P4EDITOR multi-word shell\nexpansion fix. It updates test 9805 to match the new behavior (the\nother tests also all pass).\n\nI still haven't incorporated Junio's fix for the Windows platform;\nthat's next (once I've figured out how to build git on that platform).\n\nThanks\nLuke\n\nLuke Diamand (2):\n  git-p4: add failing test for P4EDITOR handling\n  git-p4: fix handling of multi-word P4EDITOR\n\n git-p4.py                          |  2 +-\n t/t9805-git-p4-skip-submit-edit.sh |  2 +-\n t/t9820-git-p4-editor-handling.sh  | 38 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 40 insertions(+), 2 deletions(-)\n create mode 100755 t/t9820-git-p4-editor-handling.sh\n\n-- \n2.4.0.rc3.380.g8e2ddc7\n"},{"id":"261137","messageId":"1431502575-2176-2-git-send-email-luke@diamand.org","threadId":"39320","inReplyTo":"1431502575-2176-1-git-send-email-luke@diamand.org","subject":"[PATCHv2 1/2] git-p4: add failing test for P4EDITOR handling","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-05-13T07:36:14Z","receivedAt":"2015-05-13T07:36:14Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Add test case that git-p4 handles a setting of P4EDITOR\nthat takes arguments, e.g. \"gvim -f\"\n\nSigned-off-by: Luke Diamand <luke@diamand.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t9820-git-p4-editor-handling.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 1 file changed, 38 insertions(+)\n create mode 100755 t/t9820-git-p4-editor-handling.sh\n\ndiff --git a/t/t9820-git-p4-editor-handling.sh b/t/t9820-git-p4-editor-handling.sh\nnew file mode 100755\nindex 0000000..e0a3c52\n--- /dev/null\n+++ b/t/t9820-git-p4-editor-handling.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='git p4 handling of EDITOR'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+test_expect_success 'init depot' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo file1 >file1 &&\n+\t\tp4 add file1 &&\n+\t\tp4 submit -d \"file1\"\n+\t)\n+'\n+\n+test_expect_failure 'EDITOR has options' '\n+# Check that the P4EDITOR argument can be given command-line\n+# options, which git-p4 will then pass through to the shell.\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo change >file1 &&\n+\t\tgit commit -m \"change\" file1 &&\n+\t\tP4EDITOR=\"touch \\\"$git/touched\\\"\" git p4 submit &&\n+\t\ttest_path_is_file \"$git/touched\"\n+\t)\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n2.4.0.rc3.380.g8e2ddc7\n"},{"id":"261138","messageId":"1431502575-2176-3-git-send-email-luke@diamand.org","threadId":"39320","inReplyTo":"1431502575-2176-1-git-send-email-luke@diamand.org","subject":"[PATCHv2 2/2] git-p4: fix handling of multi-word P4EDITOR","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-05-13T07:36:15Z","receivedAt":"2015-05-13T07:36:15Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"This teaches git-p4 to pass the P4EDITOR variable to the\nshell for expansion, so that any command-line arguments are\ncorrectly handled. Without this, git-p4 can only launch the\neditor if P4EDITOR is solely the path to the binary, without\nany arguments.\n\nThis also fixes t9805, which relied on the previous\nbehaviour.\n\nSuggested-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Luke Diamand <luke@diamand.org>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n git-p4.py                          | 2 +-\n t/t9805-git-p4-skip-submit-edit.sh | 2 +-\n t/t9820-git-p4-editor-handling.sh  | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 41a77e6..ca6bb95 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1248,7 +1248,7 @@ class P4Submit(Command, P4UserMap):\n             editor = os.environ.get(\"P4EDITOR\")\n         else:\n             editor = read_pipe(\"git var GIT_EDITOR\").strip()\n-        system([editor, template_file])\n+        system([\"sh\", \"-c\", ('%s \"$@\"' % editor), editor, template_file])\n \n         # If the file was not saved, prompt to see if this patch should\n         # be skipped.  But skip this verification step if configured so.\ndiff --git a/t/t9805-git-p4-skip-submit-edit.sh b/t/t9805-git-p4-skip-submit-edit.sh\nindex 8931188..5fbf904 100755\n--- a/t/t9805-git-p4-skip-submit-edit.sh\n+++ b/t/t9805-git-p4-skip-submit-edit.sh\n@@ -90,7 +90,7 @@ test_expect_success 'no config, edited' '\n \t\tcd \"$git\" &&\n \t\techo line >>file1 &&\n \t\tgit commit -a -m \"change 5\" &&\n-\t\tP4EDITOR=\"$TRASH_DIRECTORY/ed.sh\" &&\n+\t\tP4EDITOR=\"\\\"$TRASH_DIRECTORY/ed.sh\\\"\" &&\n \t\texport P4EDITOR &&\n \t\tgit p4 submit &&\n \t\tp4 changes //depot/... >wc &&\ndiff --git a/t/t9820-git-p4-editor-handling.sh b/t/t9820-git-p4-editor-handling.sh\nindex e0a3c52..c178bd7 100755\n--- a/t/t9820-git-p4-editor-handling.sh\n+++ b/t/t9820-git-p4-editor-handling.sh\n@@ -17,9 +17,9 @@ test_expect_success 'init depot' '\n \t)\n '\n \n-test_expect_failure 'EDITOR has options' '\n # Check that the P4EDITOR argument can be given command-line\n # options, which git-p4 will then pass through to the shell.\n+test_expect_success 'EDITOR has options' '\n \tgit p4 clone --dest=\"$git\" //depot &&\n \ttest_when_finished cleanup_git &&\n \t(\n-- \n2.4.0.rc3.380.g8e2ddc7\n"},{"id":"261264","messageId":"xmqqzj57j7sc.fsf@gitster.dls.corp.google.com","threadId":"39320","inReplyTo":"1431502575-2176-1-git-send-email-luke@diamand.org","subject":"Re: [PATCHv2 0/2] fix handling of multi-word P4EDITOR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-14T19:09:07Z","receivedAt":"2015-05-14T19:09:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will replace.\n\nBut with a couple of minor tweaks.\n\n - s/touch/: >/ in the test added by 1/2\n - s/fixes t9850/adjusts t9805/ in the log message of 2/2\n"}]}