From 67dabab05820d97df175c724123b1232ff8d051d Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Mon, 18 Feb 2013 20:17:04 -0800 Subject: [PATCH 1/4] t/t7502: compare entire commit message with what was expected This test attempts to verify that a commit in "verbatim" mode, when supplied a commit template, produces a commit in which the commit message matches exactly the template that was supplied. But, since the commit operation appends additional instructions for the user as comments in the commit buffer, which would cause the comparison to fail, this test decided to compare only the first three lines (the length of the template) of the resulting commit message to the original template file. This has two problems. 1. It does not allow the template to be lengthened or shortened without also modifying the number of lines that are considered significant (i.e. the argument to 'head -n'). 2. It will not catch a bug in git that causes git to append additional lines to the commit message. So, let's use the --no-status option to 'git commit' which will cause git to refrain from appending the lines of instructional text to the commit message. This will allow the entire resulting commit message to be compared against the expected value. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/t7502-commit.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index cbd7a45927..9040f8ad5e 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -181,8 +181,8 @@ test_expect_success 'cleanup commit messages (verbatim option,-t)' ' echo >>negative && { echo;echo "# text";echo; } >expect && - git commit --cleanup=verbatim -t expect -a && - git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual && + git commit --cleanup=verbatim --no-status -t expect -a && + git cat-file -p HEAD |sed -e "1,/^\$/d" >actual && test_cmp expect actual ' From 5b012c80a165236dde57c386fd62465e9137b2ce Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Mon, 18 Feb 2013 20:17:05 -0800 Subject: [PATCH 2/4] t7502: demonstrate breakage with a commit message with trailing newlines This test attempts to verify that a commit message supplied to 'git commit' via the -m switch was used in full as the commit message for a commit when --cleanup=verbatim was used. But, this test has been broken since it was introduced. Since the commit message containing trailing newlines was supplied to 'git commit' using a command substitution, the trailing newlines were removed by the shell. This means that a string without any trailing newlines was actually supplied to 'git commit'. The test was able to complete successfully since internally, git appends two newlines to each string supplied via the -m switch. So, the two newlines removed by the shell were then re-added by git, and the resulting commit matched what was expected. So, let's move the initial creation of the commit message string out from within a previous test so that it stands alone. Assign the desired commit message to a variable using literal newlines. Then populate the expect file from the contents of the commit message variable. This way the shell variable becomes the authoritative source of the commit message and can be supplied via the -m switch with the trailing newlines intact. Mark this test as failing, since it is not handled correctly by git. As described above, git appends two extra newlines to every string supplied via -m, even to the ones that already end with a newline. Signed-off-by: Brandon Casey Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t7502-commit.sh | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index 9040f8ad5e..39e55f8eca 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -177,10 +177,18 @@ test_expect_success 'verbose respects diff config' ' git config --unset color.diff ' +mesg_with_comment_and_newlines=' +# text + +' + +test_expect_success 'prepare file with comment line and trailing newlines' ' + printf "%s" "$mesg_with_comment_and_newlines" >expect +' + test_expect_success 'cleanup commit messages (verbatim option,-t)' ' echo >>negative && - { echo;echo "# text";echo; } >expect && git commit --cleanup=verbatim --no-status -t expect -a && git cat-file -p HEAD |sed -e "1,/^\$/d" >actual && test_cmp expect actual @@ -196,10 +204,10 @@ test_expect_success 'cleanup commit messages (verbatim option,-F)' ' ' -test_expect_success 'cleanup commit messages (verbatim option,-m)' ' +test_expect_failure 'cleanup commit messages (verbatim option,-m)' ' echo >>negative && - git commit --cleanup=verbatim -m "$(cat expect)" -a && + git commit --cleanup=verbatim -m "$mesg_with_comment_and_newlines" -a && git cat-file -p HEAD |sed -e "1,/^\$/d">actual && test_cmp expect actual From a24a41ea9a928ccde2db074ab0835c4817223c9d Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Mon, 18 Feb 2013 20:17:06 -0800 Subject: [PATCH 3/4] git-commit: only append a newline to -m mesg if necessary Currently, git will append two newlines to every message supplied via the -m switch. The purpose of this is to allow -m to be supplied multiple times and have each supplied string become a paragraph in the resulting commit message. Normally, this does not cause a problem since any trailing newlines will be removed by the cleanup operation. If cleanup=verbatim for example, then the trailing newlines will not be removed and will survive into the resulting commit message. Instead, let's ensure that the string supplied to -m is newline terminated, but only append a second newline when appending additional messages. Fixes the test in t7502. Signed-off-by: Brandon Casey Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/commit.c | 4 +++- t/t7502-commit.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 3348aa14e9..d21d07a1a8 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -124,8 +124,10 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset) if (unset) strbuf_setlen(buf, 0); else { + if (buf->len) + strbuf_addch(buf, '\n'); strbuf_addstr(buf, arg); - strbuf_addstr(buf, "\n\n"); + strbuf_complete_line(buf); } return 0; } diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index 39e55f8eca..292bc082b2 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -204,7 +204,7 @@ test_expect_success 'cleanup commit messages (verbatim option,-F)' ' ' -test_expect_failure 'cleanup commit messages (verbatim option,-m)' ' +test_expect_success 'cleanup commit messages (verbatim option,-m)' ' echo >>negative && git commit --cleanup=verbatim -m "$mesg_with_comment_and_newlines" -a && From 46fbf7536404257a68fd4ea5d92a3bcb3f7b5848 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Tue, 19 Feb 2013 10:14:13 -0800 Subject: [PATCH 4/4] Documentation/git-commit.txt: rework the --cleanup section Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- Documentation/git-commit.txt | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 0eb79ccdba..24a99ccc99 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -172,16 +172,25 @@ OPTIONS linkgit:git-commit-tree[1]. --cleanup=:: - This option sets how the commit message is cleaned up. - The '' can be one of 'verbatim', 'whitespace', 'strip', - and 'default'. The 'default' mode will strip leading and - trailing empty lines and #commentary from the commit message - only if the message is to be edited. Otherwise only whitespace - removed. The 'verbatim' mode does not change message at all, - 'whitespace' removes just leading/trailing whitespace lines - and 'strip' removes both whitespace and commentary. The default - can be changed by the 'commit.cleanup' configuration variable - (see linkgit:git-config[1]). + This option determines how the supplied commit message should be + cleaned up before committing. The '' can be `strip`, + `whitespace`, `verbatim`, or `default`. ++ +-- +strip:: + Strip leading and trailing empty lines, trailing whitespace, and + #commentary and collapse consecutive empty lines. +whitespace:: + Same as `strip` except #commentary is not removed. +verbatim:: + Do not change the message at all. +default:: + Same as `strip` if the message is to be edited. + Otherwise `whitespace`. +-- ++ +The default can be changed by the 'commit.cleanup' configuration +variable (see linkgit:git-config[1]). -e:: --edit::