| Message ID | 1698fe0d7b7356ea1762f410767dcaf2807ea5c2.1542380865.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] commit: don't add scissors line if one exists in MERGE_MSG | expand |
Denton Liu <liu.denton@gmail.com> writes: > If commit.cleanup = scissors is specified, don't produce a scissors line > if one already exists in the MERGE_MSG file. Are we already sometimes adding a scissors line in that file? The impression I was getting was that the change in the step 2/2 is the only reason why this becomes necessary. In which case this change makes no sense as a standalone patch and requires 2/2 to be a sensible change, no? > @@ -798,7 +807,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > struct ident_split ci, ai; > > if (whence != FROM_COMMIT) { > - if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) > + if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS && > + !merge_contains_scissors) > wt_status_add_cut_line(s->fp); > status_printf_ln(s, GIT_COLOR_NORMAL, > whence == FROM_MERGE This one is done before we show a block of text, which looks correct. > @@ -824,7 +834,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > " Lines starting\nwith '%c' will be ignored, and an empty" > " message aborts the commit.\n"), comment_line_char); > else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS && > - whence == FROM_COMMIT) > + whence == FROM_COMMIT && > + !merge_contains_scissors) > wt_status_add_cut_line(s->fp); > else /* COMMIT_MSG_CLEANUP_SPACE, that is. */ > status_printf(s, GIT_COLOR_NORMAL, The correctness of this one in an if/elseif/else cascade is less clear. When "contains scissors" logic does not kick in, we just have a scissors line here (i.e. the original behaviour). Now when the new logic kicks in, we not just omit the (extra) scissors line, but also say "Please enter the commit message..." which is the message used with the "comment line char" mode of the clean-up. I wonder if this is what you really meant to have instead: > else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS && > - whence == FROM_COMMIT) > - wt_status_add_cut_line(s->fp); > + whence == FROM_COMMIT) { > + if (!merge_contains_scissors) > + wt_status_add_cut_line(s->fp); > + } > else /* COMMIT_MSG_CLEANUP_SPACE, that is. */ > status_printf(s, GIT_COLOR_NORMAL, That is, the only behaviour change in "merge contains scissors" mode is to omit the cut line and nothing else.
diff --git a/builtin/commit.c b/builtin/commit.c index 0d9828e29e..a74a324b7a 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -659,6 +659,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, const char *hook_arg2 = NULL; int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE); int old_display_comment_prefix; + int merge_contains_scissors = 0; /* This checks and barfs if author is badly specified */ determine_author_info(author_ident); @@ -719,6 +720,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, strbuf_addbuf(&sb, &message); hook_arg1 = "message"; } else if (!stat(git_path_merge_msg(the_repository), &statbuf)) { + size_t merge_msg_start; + /* * prepend SQUASH_MSG here if it exists and a * "merge --squash" was originally performed @@ -729,8 +732,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix, hook_arg1 = "squash"; } else hook_arg1 = "merge"; + + merge_msg_start = sb.len; if (strbuf_read_file(&sb, git_path_merge_msg(the_repository), 0) < 0) die_errno(_("could not read MERGE_MSG")); + + if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS && + wt_status_locate_end(sb.buf + merge_msg_start, sb.len - merge_msg_start) < sb.len - merge_msg_start) + merge_contains_scissors = 1; } else if (!stat(git_path_squash_msg(the_repository), &statbuf)) { if (strbuf_read_file(&sb, git_path_squash_msg(the_repository), 0) < 0) die_errno(_("could not read SQUASH_MSG")); @@ -798,7 +807,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, struct ident_split ci, ai; if (whence != FROM_COMMIT) { - if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) + if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS && + !merge_contains_scissors) wt_status_add_cut_line(s->fp); status_printf_ln(s, GIT_COLOR_NORMAL, whence == FROM_MERGE @@ -824,7 +834,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, " Lines starting\nwith '%c' will be ignored, and an empty" " message aborts the commit.\n"), comment_line_char); else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS && - whence == FROM_COMMIT) + whence == FROM_COMMIT && + !merge_contains_scissors) wt_status_add_cut_line(s->fp); else /* COMMIT_MSG_CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL,
If commit.cleanup = scissors is specified, don't produce a scissors line if one already exists in the MERGE_MSG file. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- builtin/commit.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)