Thread (10 messages) 10 messages, 3 authors, 2016-06-15

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

flat view

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:00:22

Possibly related (same subject, not in this thread)

On 03/18/2014 11:00 AM, Benoit Pierre wrote:
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 <redacted>
---
  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/t7514-commit-patch.sh |  4 ++--
  9 files changed, 80 insertions(+), 32 deletions(-)
[]
quoted hunk ↗ jump to hunk
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];
(Please see below)
quoted hunk ↗ jump to hunk
-	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];
Sorry being late with the review.

Recently some effort has been put to replace the
 "PATH_MAX/snprintf() combo" with code using strbuf.

So I think for new developed code it could make sense to avoid PATH_MAX from the start.
To my knowledge mkpathdup() from path.c can be used,
(or are there better ways ?)
and the whole function will look like below (not tested)
quoted hunk ↗ jump to hunk
+	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;
+}
int run_hook_with_custom_index(const char *index_file, const char *name, ...)
{
	const char *hook_env[3] =  { NULL };
	char index = mkpathdup("GIT_INDEX_FILE=%s", index_file);
	va_list args;
	int ret;

	hook_env[0] = index;

	va_start(args, name);
	ret = run_hook_ve(hook_env, name, args);
	va_end(args);

	free(index);
	return ret;
}



quoted hunk ↗ jump to hunk
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/t7514-commit-patch.sh b/t/t7514-commit-patch.sh
index 41dd37a..998a210 100755
--- a/t/t7514-commit-patch.sh
+++ b/t/t7514-commit-patch.sh
@@ -15,7 +15,7 @@ test_expect_success 'setup (initial)' '
  	git commit -m commit1
  '

-test_expect_failure 'edit hunk "commit -p -m message"' '
+test_expect_success 'edit hunk "commit -p -m message"' '
  	test_when_finished "rm -f editor_was_started" &&
  	rm -f editor_was_started &&
  	echo more >>file &&
@@ -23,7 +23,7 @@ test_expect_failure 'edit hunk "commit -p -m message"' '
  	test -r editor_was_started
  '

-test_expect_failure 'edit hunk "commit --dry-run -p -m message"' '
+test_expect_success 'edit hunk "commit --dry-run -p -m message"' '
  	test_when_finished "rm -f editor_was_started" &&
  	rm -f editor_was_started &&
  	echo more >>file &&
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help