Re: [PATCH v2 3/3] commit: add --cleanup=scissors
Duy Nguyen pclo...@gmail.com writes: On Mon, Feb 17, 2014 at 07:15:32PM +0700, Nguyễn Thái Ngọc Duy wrote: @@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, _(Please enter the commit message for your changes. Lines starting\nwith '%c' will be ignored, and an empty message aborts the commit.\n), comment_line_char); +else if (cleanup_mode == CLEANUP_SCISSORS) +wt_status_add_cut_line(s-fp); else /* CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL, _(Please enter the commit message for your changes. This cut line does not cover the merge conflict message before it. The following patch should be squashed in to move the cut line up in that case. I somehow thought that it was a policy decision we made in very early days that these conflict messages are meant to be edited with explanation of how they were resolved, not to be simply edited away? The other stuff (commented out instructions and patch text) are meant to aid humans while editing the log message, and stripping away automatically after they are done editing like your patch does is perfectly fine, but I find this change questionable. -- 8 -- diff --git a/builtin/commit.c b/builtin/commit.c index ea2912f..1033c50 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix, strbuf_addstr(committer_ident, git_committer_info(IDENT_STRICT)); if (use_editor include_status) { char *ai_tmp, *ci_tmp; - if (whence != FROM_COMMIT) + if (whence != FROM_COMMIT) { + if (cleanup_mode == CLEANUP_SCISSORS) + wt_status_add_cut_line(s-fp); status_printf_ln(s, GIT_COLOR_NORMAL, whence == FROM_MERGE ? _(\n @@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, git_path(whence == FROM_MERGE ? MERGE_HEAD : CHERRY_PICK_HEAD)); + } fprintf(s-fp, \n); if (cleanup_mode == CLEANUP_ALL) @@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, _(Please enter the commit message for your changes. Lines starting\nwith '%c' will be ignored, and an empty message aborts the commit.\n), comment_line_char); - else if (cleanup_mode == CLEANUP_SCISSORS) + else if (cleanup_mode == CLEANUP_SCISSORS whence == FROM_COMMIT) wt_status_add_cut_line(s-fp); else /* CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL, -- 8 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] commit: add --cleanup=scissors
On Tue, Feb 25, 2014 at 12:20 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Mon, Feb 17, 2014 at 07:15:32PM +0700, Nguyễn Thái Ngọc Duy wrote: @@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, _(Please enter the commit message for your changes. Lines starting\nwith '%c' will be ignored, and an empty message aborts the commit.\n), comment_line_char); +else if (cleanup_mode == CLEANUP_SCISSORS) +wt_status_add_cut_line(s-fp); else /* CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL, _(Please enter the commit message for your changes. This cut line does not cover the merge conflict message before it. The following patch should be squashed in to move the cut line up in that case. I somehow thought that it was a policy decision we made in very early days that these conflict messages are meant to be edited with explanation of how they were resolved, not to be simply edited away? The other stuff (commented out instructions and patch text) are meant to aid humans while editing the log message, and stripping away automatically after they are done editing like your patch does is perfectly fine, but I find this change questionable. I think I described it incorrectly as conflict message while it's not. This is the part to be cut out by the patch # It looks like you may be committing a merge. # If this is not correct, please remove the file. # MERGE_HEAD # and try again. (similar message for CHERRY_PICK_HEAD). -- 8 -- diff --git a/builtin/commit.c b/builtin/commit.c index ea2912f..1033c50 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix, strbuf_addstr(committer_ident, git_committer_info(IDENT_STRICT)); if (use_editor include_status) { char *ai_tmp, *ci_tmp; - if (whence != FROM_COMMIT) + if (whence != FROM_COMMIT) { + if (cleanup_mode == CLEANUP_SCISSORS) + wt_status_add_cut_line(s-fp); status_printf_ln(s, GIT_COLOR_NORMAL, whence == FROM_MERGE ? _(\n @@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, git_path(whence == FROM_MERGE ? MERGE_HEAD : CHERRY_PICK_HEAD)); + } fprintf(s-fp, \n); if (cleanup_mode == CLEANUP_ALL) @@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, _(Please enter the commit message for your changes. Lines starting\nwith '%c' will be ignored, and an empty message aborts the commit.\n), comment_line_char); - else if (cleanup_mode == CLEANUP_SCISSORS) + else if (cleanup_mode == CLEANUP_SCISSORS whence == FROM_COMMIT) wt_status_add_cut_line(s-fp); else /* CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL, -- 8 -- -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] commit: add --cleanup=scissors
Duy Nguyen pclo...@gmail.com writes: I think I described it incorrectly as conflict message while it's not. This is the part to be cut out by the patch # It looks like you may be committing a merge. # If this is not correct, please remove the file. # MERGE_HEAD # and try again. (similar message for CHERRY_PICK_HEAD). Ahh, OK. That should be removed, of course. Will squash in (but not tonight). Thanks. -- 8 -- diff --git a/builtin/commit.c b/builtin/commit.c index ea2912f..1033c50 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix, strbuf_addstr(committer_ident, git_committer_info(IDENT_STRICT)); if (use_editor include_status) { char *ai_tmp, *ci_tmp; - if (whence != FROM_COMMIT) + if (whence != FROM_COMMIT) { + if (cleanup_mode == CLEANUP_SCISSORS) + wt_status_add_cut_line(s-fp); status_printf_ln(s, GIT_COLOR_NORMAL, whence == FROM_MERGE ? _(\n @@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, git_path(whence == FROM_MERGE ? MERGE_HEAD : CHERRY_PICK_HEAD)); + } fprintf(s-fp, \n); if (cleanup_mode == CLEANUP_ALL) @@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, _(Please enter the commit message for your changes. Lines starting\nwith '%c' will be ignored, and an empty message aborts the commit.\n), comment_line_char); - else if (cleanup_mode == CLEANUP_SCISSORS) + else if (cleanup_mode == CLEANUP_SCISSORS whence == FROM_COMMIT) wt_status_add_cut_line(s-fp); else /* CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL, -- 8 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] commit: add --cleanup=scissors
On Mon, Feb 17, 2014 at 07:15:32PM +0700, Nguyễn Thái Ngọc Duy wrote: @@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, _(Please enter the commit message for your changes. Lines starting\nwith '%c' will be ignored, and an empty message aborts the commit.\n), comment_line_char); + else if (cleanup_mode == CLEANUP_SCISSORS) + wt_status_add_cut_line(s-fp); else /* CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL, _(Please enter the commit message for your changes. This cut line does not cover the merge conflict message before it. The following patch should be squashed in to move the cut line up in that case. -- 8 -- diff --git a/builtin/commit.c b/builtin/commit.c index ea2912f..1033c50 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix, strbuf_addstr(committer_ident, git_committer_info(IDENT_STRICT)); if (use_editor include_status) { char *ai_tmp, *ci_tmp; - if (whence != FROM_COMMIT) + if (whence != FROM_COMMIT) { + if (cleanup_mode == CLEANUP_SCISSORS) + wt_status_add_cut_line(s-fp); status_printf_ln(s, GIT_COLOR_NORMAL, whence == FROM_MERGE ? _(\n @@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, git_path(whence == FROM_MERGE ? MERGE_HEAD : CHERRY_PICK_HEAD)); + } fprintf(s-fp, \n); if (cleanup_mode == CLEANUP_ALL) @@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, _(Please enter the commit message for your changes. Lines starting\nwith '%c' will be ignored, and an empty message aborts the commit.\n), comment_line_char); - else if (cleanup_mode == CLEANUP_SCISSORS) + else if (cleanup_mode == CLEANUP_SCISSORS whence == FROM_COMMIT) wt_status_add_cut_line(s-fp); else /* CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL, -- 8 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] commit: add --cleanup=scissors
Since 1a72cfd (commit -v: strip diffs and submodule shortlogs from the commit message - 2013-12-05) we have a less fragile way to cut out git status at the end of a commit message but it's only enabled for stripping submodule shortlogs. Add new cleanup option that reuses the same mechanism for the entire git status without accidentally removing lines starting with '#' Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-commit.txt | 8 +++- builtin/commit.c | 9 +++-- t/t7502-commit.sh| 16 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 1a7616c..7948233 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -176,7 +176,7 @@ OPTIONS --cleanup=mode:: This option determines how the supplied commit message should be cleaned up before committing. The 'mode' can be `strip`, - `whitespace`, `verbatim`, or `default`. + `whitespace`, `verbatim`, `scissors` or `default`. + -- strip:: @@ -186,6 +186,12 @@ whitespace:: Same as `strip` except #commentary is not removed. verbatim:: Do not change the message at all. +scissors:: + Same as `whitespace`, except that everything from (and + including) the line + `# 8 ` + is truncated if the message is to be edited. `#` can be + customized with core.commentChar. default:: Same as `strip` if the message is to be edited. Otherwise `whitespace`. diff --git a/builtin/commit.c b/builtin/commit.c index 3767478..ea2912f 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -113,6 +113,7 @@ static char *sign_commit; static enum { CLEANUP_SPACE, CLEANUP_NONE, + CLEANUP_SCISSORS, CLEANUP_ALL } cleanup_mode; static const char *cleanup_arg; @@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, _(Please enter the commit message for your changes. Lines starting\nwith '%c' will be ignored, and an empty message aborts the commit.\n), comment_line_char); + else if (cleanup_mode == CLEANUP_SCISSORS) + wt_status_add_cut_line(s-fp); else /* CLEANUP_SPACE, that is. */ status_printf(s, GIT_COLOR_NORMAL, _(Please enter the commit message for your changes. @@ -1132,6 +1135,8 @@ static int parse_and_validate_options(int argc, const char *argv[], cleanup_mode = CLEANUP_SPACE; else if (!strcmp(cleanup_arg, strip)) cleanup_mode = CLEANUP_ALL; + else if (!strcmp(cleanup_arg, scissors)) + cleanup_mode = use_editor ? CLEANUP_SCISSORS : CLEANUP_SPACE; else die(_(Invalid cleanup mode %s), cleanup_arg); @@ -1600,8 +1605,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) die(_(could not read commit message: %s), strerror(saved_errno)); } - /* Truncate the message just before the diff, if any. */ - if (verbose) + if (verbose || /* Truncate the message just before the diff, if any. */ + cleanup_mode == CLEANUP_SCISSORS) wt_status_truncate_message_at_cut_line(sb); if (cleanup_mode != CLEANUP_NONE) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index 6313da2..9a3f3a1 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -223,6 +223,22 @@ test_expect_success 'cleanup commit messages (whitespace option,-F)' ' ' +test_expect_success 'cleanup commit messages (scissors option,-F,-e)' ' + + echo negative + cat text EOF + +# to be kept +# 8 +to be removed +EOF + echo # to be kept expect + git commit --cleanup=scissors -e -F text -a + git cat-file -p HEAD |sed -e 1,/^\$/dactual + test_cmp expect actual + +' + test_expect_success 'cleanup commit messages (strip option,-F)' ' echo negative -- 1.8.5.2.240.g8478abd -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html