Re: [PATCH 2/2] Teach commit about CHERRY_PICK_HEAD
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:34
Jay Soffian wrote:
This change fixes that situation. A bare 'commit' will now take the authorship from CHERRY_PICK_HEAD and the commit message from MERGE_MSG. If the user wishes to reset authorship, that must now be done explicitly via --reset-author.
Might also be worth mentioning that it makes --amend fail in such a situation (a change worth celebrating).
quoted hunk ↗ jump to hunk
--- a/builtin/commit.c +++ b/builtin/commit.c@@ -56,8 +56,10 @@ static const char empty_amend_advice[] = static unsigned char head_sha1[20]; -static char *use_message_buffer; +static const char *use_message_buffer; static const char commit_editmsg[] = "COMMIT_EDITMSG"; +static const char cherry_pick_head[] = "CHERRY_PICK_HEAD"; +static const char merge_head[] = "MERGE_HEAD";
Hmm, these variables but not MERGE_MSG, MERGE_MODE, and SQUASH_MSG?
quoted hunk ↗ jump to hunk
@@ -68,6 +70,7 @@ static enum { static const char *logfile, *force_author; static const char *template_file; +static const char *author_message, *author_message_buffer;
That's not a message at all, is it? On first reading I thought it would be a message about the author. Maybe a comment can help. /* name and content of commit from which to copy authorship */
quoted hunk ↗ jump to hunk
@@ -88,7 +91,8 @@ static enum { } cleanup_mode; static char *cleanup_arg; -static int use_editor = 1, initial_commit, in_merge, include_status = 1; +static enum commit_whence whence; +static int use_editor = 1, initial_commit, include_status = 1;
The name "whence" is not so self-explanatory but I don't have any better ideas (I probably would have written "merge_or_cherry_pick"; we can be glad you came up with something better).
quoted hunk ↗ jump to hunk
@@ -163,6 +167,36 @@ static struct option builtin_commit_options[] = { OPT_END() }; +static void determine_whence(struct wt_status *s) +{ + if (file_exists(git_path(merge_head))) + whence = FROM_MERGE;
Micronit: maybe COMMITTING_A_MERGE or COMMIT_DURING_MERGE to avoid using valuable namespace?
quoted hunk ↗ jump to hunk
@@ -644,7 +678,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, * This final case does not modify the template message, * it just sets the argument to the prepare-commit-msg hook. */ - else if (in_merge) + else if (whence == FROM_MERGE) hook_arg1 = "merge";
Perhaps:
else if (whence == CHERRY_PICK) {
hook_arg1 = "commit";
hook_arg2 = author_message;
}
quoted hunk ↗ jump to hunk
@@ -694,16 +728,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
[...]
"# If this is not correct, please remove the file\n" "# %s\n" "# and try again.\n" "#\n", + whence_s(), + git_path(whence == FROM_MERGE + ? merge_head + : cherry_pick_head));
Ok. We probably should move away from having to suggest "rm -f .git/whatever" in the future (maybe git update-ref -d %s is simpler advice? I dunno).
quoted hunk ↗ jump to hunk
@@ -898,6 +934,27 @@ static void handle_untracked_files_arg(struct wt_status *s) die("Invalid untracked files mode '%s'", untracked_files_arg); } +static const char *read_commit_message(const char *name) {
Nice. Opening '{' should be in the first column.
quoted hunk ↗ jump to hunk
+++ b/builtin/revert.c
[...]
quoted hunk ↗ jump to hunk
@@ -284,9 +233,7 @@ static void print_advice(void) advise("after resolving the conflicts, mark the corrected paths"); advise("with 'git add <paths>' or 'git rm <paths>'"); - - if (action == CHERRY_PICK) - advise("and commit the result with 'git commit -c CHERRY_PICK_HEAD'"); + advise("and commit the result with 'git commit'");
Hoorah!
quoted hunk ↗ jump to hunk
--- a/wt-status.c +++ b/wt-status.c@@ -60,7 +60,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s) color_fprintf_ln(s->fp, c, "# Unmerged paths:"); if (!advice_status_hints) return; - if (s->in_merge) + if (s->whence != FROM_COMMIT) ; else if (!s->is_initial) color_fprintf_ln(s->fp, c, "# (use \"git reset %s <file>...\" to unstage)", s->reference);
Isn't the advice of using "git reset -- <paths>" still good in the CHERRY_PICK case?
quoted hunk ↗ jump to hunk
@@ -77,7 +77,7 @@ static void wt_status_print_cached_header(struct wt_status *s) color_fprintf_ln(s->fp, c, "# Changes to be committed:"); if (!advice_status_hints) return; - if (s->in_merge) + if (s->whence != FROM_COMMIT) ; /* NEEDSWORK: use "git reset --unresolve"??? */
Likewise here.
quoted hunk ↗ jump to hunk
--- a/wt-status.h +++ b/wt-status.h@@ -24,6 +24,12 @@ enum untracked_status_type { SHOW_ALL_UNTRACKED_FILES }; +enum commit_whence { + FROM_COMMIT, + FROM_MERGE, + FROM_CHERRY_PICK +};
Style: please use tabs to indent.
quoted hunk ↗ jump to hunk
@@ -40,7 +46,7 @@ struct wt_status { const char **pathspec; int verbose; int amend; - int in_merge; + enum commit_whence whence;
Might benefit from a comment. /* whether a merge or cherry-pick is in progress */ enum commit_whence whence; Thanks, very readable.