Merge branch 'bp/commit-p-editor'

When it is not necessary to edit a commit log message (e.g. "git
commit -m" is given a message without specifying "-e"), we used to
disable the spawning of the editor by overriding GIT_EDITOR, but
this means all the uses of the editor, other than to edit the
commit log message, are also affected.

* bp/commit-p-editor:
  run-command: mark run_hook_with_custom_index as deprecated
  merge hook tests: fix and update tests
  merge: fix GIT_EDITOR override for commit hook
  commit: fix patch hunk editing with "commit -p -m"
  test patch hunk editing with "commit -p -m"
  merge hook tests: use 'test_must_fail' instead of '!'
  merge hook tests: fix missing '&&' in test
This commit is contained in:
Junio C Hamano 2014-03-28 13:51:11 -07:00
commit 9abf65d23c
10 changed files with 137 additions and 39 deletions

View file

@ -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. */

View file

@ -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);

View file

@ -611,7 +611,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) {
@ -873,8 +873,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) {
@ -890,7 +890,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;
}
@ -1074,8 +1074,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)
@ -1458,6 +1456,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;
@ -1682,7 +1703,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");

View file

@ -184,7 +184,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;
}

View file

@ -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);
}
@ -824,8 +824,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(0 < option_edit, 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))

View file

@ -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 */

View file

@ -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;
}

View file

@ -47,7 +47,12 @@ 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
__attribute__((deprecated))
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 */

View file

@ -134,14 +134,26 @@ test_expect_success 'with hook (-c)' '
test_expect_success 'with hook (merge)' '
head=`git rev-parse HEAD` &&
git checkout -b other HEAD@{1} &&
echo "more" >> file &&
test_when_finished "git checkout -f master" &&
git checkout -B other HEAD@{1} &&
echo "more" >>file &&
git add file &&
git commit -m other &&
git checkout - &&
git merge other &&
test "`git log -1 --pretty=format:%s`" = merge
git merge --no-ff other &&
test "`git log -1 --pretty=format:%s`" = "merge (no editor)"
'
test_expect_success 'with hook and editor (merge)' '
test_when_finished "git checkout -f master" &&
git checkout -B other HEAD@{1} &&
echo "more" >>file &&
git add file &&
git commit -m other &&
git checkout - &&
env GIT_EDITOR="\"\$FAKE_EDITOR\"" git merge --no-ff -e other &&
test "`git log -1 --pretty=format:%s`" = "merge"
'
cat > "$HOOK" <<'EOF'
@ -151,34 +163,37 @@ EOF
test_expect_success 'with failing hook' '
test_when_finished "git checkout -f master" &&
head=`git rev-parse HEAD` &&
echo "more" >> file &&
git add file &&
! GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -c $head
test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -c $head
'
test_expect_success 'with failing hook (--no-verify)' '
test_when_finished "git checkout -f master" &&
head=`git rev-parse HEAD` &&
echo "more" >> file &&
git add file &&
! GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --no-verify -c $head
test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --no-verify -c $head
'
test_expect_success 'with failing hook (merge)' '
test_when_finished "git checkout -f master" &&
git checkout -B other HEAD@{1} &&
echo "more" >> file &&
git add file &&
rm -f "$HOOK" &&
git commit -m other &&
write_script "$HOOK" <<-EOF
write_script "$HOOK" <<-EOF &&
exit 1
EOF
git checkout - &&
test_must_fail git merge other
test_must_fail git merge --no-ff other
'

34
t/t7514-commit-patch.sh Executable file
View file

@ -0,0 +1,34 @@
#!/bin/sh
test_description='hunk edit with "commit -p -m"'
. ./test-lib.sh
if ! test_have_prereq PERL
then
skip_all="skipping '$test_description' tests, perl not available"
test_done
fi
test_expect_success 'setup (initial)' '
echo line1 >file &&
git add file &&
git commit -m commit1
'
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 &&
echo e | env GIT_EDITOR=": >editor_was_started" git commit -p -m commit2 file &&
test -r editor_was_started
'
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 &&
echo e | env GIT_EDITOR=": >editor_was_started" git commit -p -m commit3 file &&
test -r editor_was_started
'
test_done