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

[PATCH 4/7] commit: fix patch hunk editing with "commit -p -m"

From
Benoit Pierre <benoit.pierre@gmail.com>
Date
Mar 10, 2014, 18:49 UTC
Message-ID
<1394477377-10994-5-git-send-email-benoit.pierre@gmail.com>
In-Reply-To
<1394477377-10994-1-git-send-email-benoit.pierre@gmail.com>

Don't change git environment: move the GIT_EDITOR=":" override to the hook command subprocess, like it's already done for GIT_INDEX_FILE.

Signed-off-by: Benoit Pierre <benoit.pierre@gmail.com>
---
 builtin/checkout.c                |  8 +++----
 builtin/clone.c                   |  4 ++--
 builtin/commit.c                  | 35 ++++++++++++++++++++++++-------
 builtin/gc.c                      |  2 +-
 builtin/merge.c                   |  6 +++---
 commit.h                          |  3 +++
 run-command.c                     | 44 ++++++++++++++++++++++++++++-----------
 run-command.h                     |  6 +++++-
 t/t7513-commit_-p_-m_hunk_edit.sh |  2 +-
 9 files changed, 79 insertions(+), 31 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5df3837..da423b2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -53,10 +53,10 @@ struct checkout_opts {
 static int post_checkout_hook(struct commit *old, struct commit *new,
 			      int changed)
 {
-	return run_hook(NULL, "post-checkout",
-			sha1_to_hex(old ? old->object.sha1 : null_sha1),
-			sha1_to_hex(new ? new->object.sha1 : null_sha1),
-			changed ? "1" : "0", NULL);
+    return run_hook_le(NULL, "post-checkout",
+		       sha1_to_hex(old ? old->object.sha1 : null_sha1),
+		       sha1_to_hex(new ? new->object.sha1 : null_sha1),
+		       changed ? "1" : "0", NULL);
 	/* "new" can be NULL when checking out from the index before
 	   a commit exists. */
 
diff --git a/builtin/clone.c b/builtin/clone.c
index 43e772c..9b3c04d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -660,8 +660,8 @@ static int checkout(void)
 	    commit_locked_index(lock_file))
 		die(_("unable to write new index file"));
 
-	err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
-			sha1_to_hex(sha1), "1", NULL);
+	err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
+			   sha1_to_hex(sha1), "1", NULL);
 
 	if (!err && option_recursive)
 		err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
diff --git a/builtin/commit.c b/builtin/commit.c
index 3767478..baf1fc0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -612,7 +612,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	/* This checks and barfs if author is badly specified */
 	determine_author_info(author_ident);
 
-	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
+	if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
 		return 0;
 
 	if (squash_message) {
@@ -866,8 +866,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		return 0;
 	}
 
-	if (run_hook(index_file, "prepare-commit-msg",
-		     git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
+	if (run_commit_hook(use_editor, index_file, "prepare-commit-msg",
+			    git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
 		return 0;
 
 	if (use_editor) {
@@ -883,7 +883,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 
 	if (!no_verify &&
-	    run_hook(index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
+	    run_commit_hook(use_editor, index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
 		return 0;
 	}
 
@@ -1067,8 +1067,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		use_editor = 0;
 	if (0 <= edit_flag)
 		use_editor = edit_flag;
-	if (!use_editor)
-		setenv("GIT_EDITOR", ":", 1);
 
 	/* Sanity check options */
 	if (amend && !current_head)
@@ -1445,6 +1443,29 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
 	return finish_command(&proc);
 }
 
+int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...)
+{
+	const char *hook_env[3] =  { NULL };
+	char index[PATH_MAX];
+	va_list args;
+	int ret;
+
+	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+	hook_env[0] = index;
+
+	/*
+	 * Let the hook know that no editor will be launched.
+	 */
+	if (!editor_is_used)
+		hook_env[1] = "GIT_EDITOR=:";
+
+	va_start(args, name);
+	ret = run_hook_ve(hook_env, name, args);
+	va_end(args);
+
+	return ret;
+}
+
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
 	static struct wt_status s;
@@ -1669,7 +1690,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		     "not exceeded, and then \"git reset HEAD\" to recover."));
 
 	rerere(0);
-	run_hook(get_index_file(), "post-commit", NULL);
+	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
 		struct notes_rewrite_cfg *cfg;
 		cfg = init_copy_notes_for_rewrite("amend");
diff --git a/builtin/gc.c b/builtin/gc.c
index c19545d..7fa717a 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -179,7 +179,7 @@ static int need_to_gc(void)
 	else if (!too_many_loose_objects())
 		return 0;
 
-	if (run_hook(NULL, "pre-auto-gc", NULL))
+	if (run_hook_le(NULL, "pre-auto-gc", NULL))
 		return 0;
 	return 1;
 }
diff --git a/builtin/merge.c b/builtin/merge.c
index e576a7f..67f312d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -421,7 +421,7 @@ static void finish(struct commit *head_commit,
 	}
 
 	/* Run a post-merge hook */
-	run_hook(NULL, "post-merge", squash ? "1" : "0", NULL);
+	run_hook_le(NULL, "post-merge", squash ? "1" : "0", NULL);
 
 	strbuf_release(&reflog_message);
 }
@@ -821,8 +821,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	if (0 < option_edit)
 		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
 	write_merge_msg(&msg);
-	if (run_hook(get_index_file(), "prepare-commit-msg",
-		     git_path("MERGE_MSG"), "merge", NULL, NULL))
+	if (run_commit_hook(1, get_index_file(), "prepare-commit-msg",
+			    git_path("MERGE_MSG"), "merge", NULL))
 		abort_commit(remoteheads, NULL);
 	if (0 < option_edit) {
 		if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
diff --git a/commit.h b/commit.h
index 16d9c43..8d97a5c 100644
--- a/commit.h
+++ b/commit.h
@@ -304,4 +304,7 @@ extern void check_commit_signature(const struct commit* commit, struct signature
 
 int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused);
 
+LAST_ARG_MUST_BE_NULL
+extern int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);
+
 #endif /* COMMIT_H */
diff --git a/run-command.c b/run-command.c
index 3914d9c..75abc47 100644
--- a/run-command.c
+++ b/run-command.c
@@ -760,13 +760,11 @@ char *find_hook(const char *name)
 	return path;
 }
 
-int run_hook(const char *index_file, const char *name, ...)
+int run_hook_ve(const char *const *env, const char *name, va_list args)
 {
 	struct child_process hook;
 	struct argv_array argv = ARGV_ARRAY_INIT;
-	const char *p, *env[2];
-	char index[PATH_MAX];
-	va_list args;
+	const char *p;
 	int ret;
 
 	p = find_hook(name);
@@ -775,23 +773,45 @@ int run_hook(const char *index_file, const char *name, ...)
 
 	argv_array_push(&argv, p);
 
-	va_start(args, name);
 	while ((p = va_arg(args, const char *)))
 		argv_array_push(&argv, p);
-	va_end(args);
 
 	memset(&hook, 0, sizeof(hook));
 	hook.argv = argv.argv;
+	hook.env = env;
 	hook.no_stdin = 1;
 	hook.stdout_to_stderr = 1;
-	if (index_file) {
-		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
-		env[0] = index;
-		env[1] = NULL;
-		hook.env = env;
-	}
 
 	ret = run_command(&hook);
 	argv_array_clear(&argv);
 	return ret;
 }
+
+int run_hook_le(const char *const *env, const char *name, ...)
+{
+	va_list args;
+	int ret;
+
+	va_start(args, name);
+	ret = run_hook_ve(env, name, args);
+	va_end(args);
+
+	return ret;
+}
+
+int run_hook_with_custom_index(const char *index_file, const char *name, ...)
+{
+	const char *hook_env[3] =  { NULL };
+	char index[PATH_MAX];
+	va_list args;
+	int ret;
+
+	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+	hook_env[0] = index;
+
+	va_start(args, name);
+	ret = run_hook_ve(hook_env, name, args);
+	va_end(args);
+
+	return ret;
+}
diff --git a/run-command.h b/run-command.h
index 6b985af..88460f9 100644
--- a/run-command.h
+++ b/run-command.h
@@ -47,7 +47,11 @@ int run_command(struct child_process *);
 
 extern char *find_hook(const char *name);
 LAST_ARG_MUST_BE_NULL
-extern int run_hook(const char *index_file, const char *name, ...);
+extern int run_hook_le(const char *const *env, const char *name, ...);
+extern int run_hook_ve(const char *const *env, const char *name, va_list args);
+
+LAST_ARG_MUST_BE_NULL
+extern int run_hook_with_custom_index(const char *index_file, const char *name, ...);
 
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh
index 994939a..1f05d32 100755
--- a/t/t7513-commit_-p_-m_hunk_edit.sh
+++ b/t/t7513-commit_-p_-m_hunk_edit.sh
@@ -25,7 +25,7 @@ test_expect_success 'setup (initial)' '
 	EOF
 '
 
-test_expect_failure 'edit hunk "commit -p -m message"' '
+test_expect_success 'edit hunk "commit -p -m message"' '
 	echo e | env GIT_EDITOR="sed s/+line3\$/+line2/ -i" git commit -p -m commit2 file &&
 	git diff HEAD^ HEAD >actual &&
 	test_cmp expect actual

-- 
1.9.0
Previous: Benoit PierreNext: Jeff King
Message 37 of 50 in “fix hunk editing with 'commit -p -m'”
  1. 0/7 fix hunk editing with 'commit -p -m'Benoit Pierre, Mar 10, 2014
  2. 1/7 merge hook tests: fix missing '&&' in testBenoit Pierre, Mar 10, 2014
  3. 2/7 merge hook tests: use 'test_must_fail' instead of '!'Benoit Pierre, Mar 10, 2014
  4. 3/7 test patch hunk editing with "commit -p -m"Benoit Pierre, Mar 10, 2014
  5. Eric SunshineMar 10, 2014
  6. Junio C HamanoMar 11, 2014
  7. Philip OakleyMar 10, 2014
  8. Junio C HamanoMar 11, 2014
  9. Torsten BögershausenMar 15, 2014
  10. Benoit PierreMar 15, 2014
  11. Torsten BögershausenMar 15, 2014
  12. 1/7 merge hook tests: fix missing '&&' in testBenoit Pierre, Mar 15, 2014
  13. 2/7 merge hook tests: use 'test_must_fail' instead of '!'Benoit Pierre, Mar 15, 2014
  14. 3/7 test patch hunk editing with "commit -p -m"Benoit Pierre, Mar 15, 2014
  15. Junio C HamanoMar 17, 2014
  16. Benoit PierreMar 17, 2014
  17. Eric SunshineMar 17, 2014
  18. Benoit PierreMar 17, 2014
  19. Junio C HamanoMar 17, 2014
  20. Junio C HamanoMar 17, 2014
  21. Benoit PierreMar 17, 2014
  22. Junio C HamanoMar 17, 2014
  23. 1/7 merge hook tests: fix missing '&&' in testBenoit Pierre, Mar 18, 2014
  24. 2/7 merge hook tests: use 'test_must_fail' instead of '!'Benoit Pierre, Mar 18, 2014
  25. 3/7 test patch hunk editing with "commit -p -m"Benoit Pierre, Mar 18, 2014
  26. 4/7 commit: fix patch hunk editing with "commit -p -m"Benoit Pierre, Mar 18, 2014
  27. Torsten BögershausenMar 19, 2014
  28. Junio C HamanoMar 19, 2014
  29. 5/7 merge: fix GIT_EDITOR override for commit hookBenoit Pierre, Mar 18, 2014
  30. 6/7 merge hook tests: fix and update testsBenoit Pierre, Mar 18, 2014
  31. 7/7 run-command: mark run_hook_with_custom_index as deprecatedBenoit Pierre, Mar 18, 2014
  32. Junio C HamanoMar 18, 2014
  33. 4/7 commit: fix patch hunk editing with "commit -p -m"Benoit Pierre, Mar 15, 2014
  34. 5/7 merge: fix GIT_EDITOR override for commit hookBenoit Pierre, Mar 15, 2014
  35. 6/7 merge hook tests: fix and update testsBenoit Pierre, Mar 15, 2014
  36. 7/7 run-command: mark run_hook_with_custom_index as deprecatedBenoit Pierre, Mar 15, 2014
  37. 4/7 commit: fix patch hunk editing with "commit -p -m"Benoit Pierre, Mar 10, 2014
  38. Jeff KingMar 10, 2014
  39. Jun HaoMar 11, 2014
  40. Benoit PierreMar 11, 2014
  41. Jeff KingMar 11, 2014
  42. Jun HaoMar 11, 2014
  43. Junio C HamanoMar 11, 2014
  44. 5/7 merge: fix GIT_EDITOR override for commit hookBenoit Pierre, Mar 10, 2014
  45. 6/7 merge hook tests: fix and update testsBenoit Pierre, Mar 10, 2014
  46. Eric SunshineMar 10, 2014
  47. 7/7 run-command: mark run_hook_with_custom_index as deprecatedBenoit Pierre, Mar 10, 2014
  48. brian m. carlsonMar 11, 2014
  49. Benoit PierreMar 11, 2014
  50. Jonathan NiederApr 3, 2014

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.