git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCHv3 2/3] git-p4: fix handling of multi-word P4EDITOR

From
Luke Diamand <luke@diamand.org>
Date
May 19, 2015, 22:23 UTC
Message-ID
<1432074198-13806-3-git-send-email-luke@diamand.org>
In-Reply-To
<1432074198-13806-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 adjusts t9805, which relied on the previous behaviour.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-p4.py                          | 2 +-
 t/t9805-git-p4-skip-submit-edit.sh | 2 +-
 t/t9820-git-p4-editor-handling.sh  | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index ff132b2..de06046 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([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 af924bb..7026ef9 100755
--- a/t/t9820-git-p4-editor-handling.sh
+++ b/t/t9820-git-p4-editor-handling.sh
@@ -17,7 +17,6 @@ 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' '
-- 
2.4.1.502.gb11c5ab
Previous: Luke DiamandNext: Luke Diamand
Message 8 of 11 in “git-p4: test t9820-git-p4-editor-handling.sh failing”
  1. Luke DiamandMay 19, 2015
  2. Junio C HamanoMay 19, 2015
  3. 0/3 git-p4: fix handling of multi-word P4EDITORLuke Diamand, May 19, 2015
  4. 1/3 git-p4: add failing test for P4EDITOR handlingLuke Diamand, May 19, 2015
  5. Junio C HamanoMay 20, 2015
  6. Junio C HamanoMay 20, 2015
  7. Luke DiamandMay 20, 2015
  8. 2/3 git-p4: fix handling of multi-word P4EDITORLuke Diamand, May 19, 2015
  9. 3/3 git-p4: tests: use test-chmtime in place of touchLuke Diamand, May 19, 2015
  10. Luke DiamandMay 19, 2015
  11. Junio C HamanoMay 24, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.