From 1a72cfd7fa88e5a6c2b94568ac2fe69dfbd87f87 Mon Sep 17 00:00:00 2001 From: Jens Lehmann Date: Thu, 5 Dec 2013 20:44:14 +0100 Subject: commit -v: strip diffs and submodule shortlogs from the commit message When using the '-v' option of "git commit" the diff added to the commit message temporarily for editing is stripped off after the user exited the editor by searching for "\ndiff --git " and truncating the commmit message there if it is found. But this approach has two problems: - when the commit message itself contains a line starting with "diff --git" it will be truncated there prematurely; and - when the "diff.submodule" setting is set to "log", the diff may start with "Submodule ..", which will be left in the commit message while it shouldn't. Fix that by introducing a special scissor separator line starting with the comment character ('#' or the core.commentChar config if set) followed by two lines describing what it is for. The scissor line - which will not be translated - is used to reliably detect the start of the diff so it can be chopped off from the commit message, no matter what the user enters there. Turn a known test failure fixed by this change into a successful test; also add one for a diff starting with a submodule log and another one for proper handling of the comment char. Reported-by: Ari Pollak Signed-off-by: Jens Lehmann Signed-off-by: Junio C Hamano --- builtin/commit.c | 9 +++------ t/t7507-commit-verbose.sh | 28 +++++++++++++++++++++++++++- wt-status.c | 29 +++++++++++++++++++++++++++-- wt-status.h | 1 + 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 6ab4605cf5..fedb45af8c 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1505,7 +1505,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) struct strbuf sb = STRBUF_INIT; struct strbuf author_ident = STRBUF_INIT; const char *index_file, *reflog_msg; - char *nl, *p; + char *nl; unsigned char sha1[20]; struct ref_lock *ref_lock; struct commit_list *parents = NULL, **pptr = &parents; @@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) } /* Truncate the message just before the diff, if any. */ - if (verbose) { - p = strstr(sb.buf, "\ndiff --git "); - if (p != NULL) - strbuf_setlen(&sb, p - sb.buf + 1); - } + if (verbose) + wt_status_truncate_message_at_cut_line(&sb); if (cleanup_mode != CLEANUP_NONE) stripspace(&sb, cleanup_mode == CLEANUP_ALL); diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh index da5bd3b5a5..2ddf28c984 100755 --- a/t/t7507-commit-verbose.sh +++ b/t/t7507-commit-verbose.sh @@ -65,9 +65,35 @@ test_expect_success 'diff in message is retained without -v' ' check_message diff ' -test_expect_failure 'diff in message is retained with -v' ' +test_expect_success 'diff in message is retained with -v' ' git commit --amend -F diff -v && check_message diff ' +test_expect_success 'submodule log is stripped out too with -v' ' + git config diff.submodule log && + git submodule add ./. sub && + git commit -m "sub added" && + ( + cd sub && + echo "more" >>file && + git commit -a -m "submodule commit" + ) && + ( + GIT_EDITOR=cat && + export GIT_EDITOR && + test_must_fail git commit -a -v 2>err + ) && + test_i18ngrep "Aborting commit due to empty commit message." err +' + +test_expect_success 'verbose diff is stripped out with set core.commentChar' ' + ( + GIT_EDITOR=cat && + export GIT_EDITOR && + test_must_fail git -c core.commentchar=";" commit -a -v 2>err + ) && + test_i18ngrep "Aborting commit due to empty commit message." err +' + test_done diff --git a/wt-status.c b/wt-status.c index b4e44baa29..85390b813f 100644 --- a/wt-status.c +++ b/wt-status.c @@ -16,6 +16,9 @@ #include "column.h" #include "strbuf.h" +static char cut_line[] = +"------------------------ >8 ------------------------\n"; + static char default_wt_status_colors[][COLOR_MAXLEN] = { GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */ GIT_COLOR_GREEN, /* WT_STATUS_UPDATED */ @@ -767,6 +770,18 @@ conclude: status_printf_ln(s, GIT_COLOR_NORMAL, ""); } +void wt_status_truncate_message_at_cut_line(struct strbuf *buf) +{ + const char *p; + struct strbuf pattern = STRBUF_INIT; + + strbuf_addf(&pattern, "%c %s", comment_line_char, cut_line); + p = strstr(buf->buf, pattern.buf); + if (p && (p == buf->buf || p[-1] == '\n')) + strbuf_setlen(buf, p - buf->buf); + strbuf_release(&pattern); +} + static void wt_status_print_verbose(struct wt_status *s) { struct rev_info rev; @@ -787,10 +802,20 @@ static void wt_status_print_verbose(struct wt_status *s) * If we're not going to stdout, then we definitely don't * want color, since we are going to the commit message * file (and even the "auto" setting won't work, since it - * will have checked isatty on stdout). + * will have checked isatty on stdout). But we then do want + * to insert the scissor line here to reliably remove the + * diff before committing. */ - if (s->fp != stdout) + if (s->fp != stdout) { + const char *explanation = _("Do not touch the line above.\nEverything below will be removed."); + struct strbuf buf = STRBUF_INIT; + rev.diffopt.use_color = 0; + fprintf(s->fp, "%c %s", comment_line_char, cut_line); + strbuf_add_commented_lines(&buf, explanation, strlen(explanation)); + fputs(buf.buf, s->fp); + strbuf_release(&buf); + } run_diff_index(&rev, 1); } diff --git a/wt-status.h b/wt-status.h index 6c29e6f5e5..30a4812583 100644 --- a/wt-status.h +++ b/wt-status.h @@ -91,6 +91,7 @@ struct wt_status_state { unsigned char cherry_pick_head_sha1[20]; }; +void wt_status_truncate_message_at_cut_line(struct strbuf *); void wt_status_prepare(struct wt_status *s); void wt_status_print(struct wt_status *s); void wt_status_collect(struct wt_status *s); -- cgit v1.2.1