From: Phillip Wood Date: Mon, 06 Jan 2020 17:10:46 GMT Subject: Re: [PATCH 1/1] sequencer: comment out the 'squash!' line Message-ID: <13b47c13-7a8b-a205-0cdb-5fdcb8d42412@gmail.com> In-Reply-To: Hi Michael On 06/01/2020 16:04, Michael Rappazzo via GitGitGadget wrote: > From: Michael Rappazzo > > When performing a squash commit, the commit comments are combined into a > single commit. Since the subject line of the squash commit is used to > identify the squash-to target commit, it cannot offer any useful > contribution to the new commit message. Therefore, the squash commit > subject line it commented out from the combined message (much like a > fixup commit's full comment). I like the idea but I think it would be better to only comment out the subject of the commit message if it starts with squash! for fixup! otherwise it may well be a useful part of the message. For correctness I think it would be better to comment out the subject (everything before the first blank line as returned by `git log --pretty=%s`) rather than just the first line. I've actually implemented this as part of a longer series that I've never got round to posting to the list[1] - feel free to use any of that which you find useful. That commit also shows an alternative way to change the --autosquash tests. [1] https://github.com/phillipwood/git/commit/b91b492e4aba1ac8d244859361379d5063cfc2b8 > The body of a squash commit may contain additional content to add to the > commit message, so this part of the squash commit message is retained. > > Since this change what the expected post-rebase commit comment would look > like, related test expectations are adjusted to reflect the the new > expectation. A new test is added for the new expectation. > > Signed-off-by: Michael Rappazzo > --- > sequencer.c | 1 + > t/t3404-rebase-interactive.sh | 4 +--- > t/t3415-rebase-autosquash.sh | 36 +++++++++++++++++++++++++++-------- > t/t3900-i18n-commit.sh | 4 ---- > 4 files changed, 30 insertions(+), 15 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index 763ccbbc45..e5602686d7 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1756,6 +1756,7 @@ static int update_squash_messages(struct repository *r, > strbuf_addf(&buf, _("This is the commit message #%d:"), > ++opts->current_fixup_count + 1); > strbuf_addstr(&buf, "\n\n"); > + strbuf_addf(&buf, "%c ", comment_line_char); > strbuf_addstr(&buf, body); > } else if (command == TODO_FIXUP) { > strbuf_addf(&buf, "\n%c ", comment_line_char); > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index ae6e55ce79..57d178d431 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -513,8 +513,6 @@ test_expect_success C_LOCALE_OUTPUT 'squash and fixup generate correct log messa > cat >expect-squash-fixup <<-\EOF && > B > > - D > - > ONCE > EOF > git checkout -b squash-fixup E && > @@ -1325,7 +1323,7 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)' > test_cmp_rev HEAD F && > rm file6 && > git rebase --continue && > - test $(git cat-file commit HEAD | sed -ne \$p) = I && > + test $(git cat-file commit HEAD | sed -ne \$p) = F && > git reset --hard original-branch2 > ' > > diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh > index 22d218698e..51c5a94aea 100755 > --- a/t/t3415-rebase-autosquash.sh > +++ b/t/t3415-rebase-autosquash.sh > @@ -59,7 +59,6 @@ test_auto_squash () { > git add -u && > test_tick && > git commit -m "squash! first" && > - > git tag $1 && > test_tick && > git rebase $2 -i HEAD^^^ && > @@ -67,7 +66,7 @@ test_auto_squash () { > test_line_count = 3 actual && > git diff --exit-code $1 && > test 1 = "$(git cat-file blob HEAD^:file1)" && > - test 2 = $(git cat-file commit HEAD^ | grep first | wc -l) > + test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) > } > > test_expect_success 'auto squash (option)' ' > @@ -82,6 +81,27 @@ test_expect_success 'auto squash (config)' ' > test_must_fail test_auto_squash final-squash-config-false > ' > > +test_expect_success 'auto squash includes squash body but not squash directive' ' > + git reset --hard base && > + echo 1 >file1 && > + git add -u && > + test_tick && > + git commit -m "squash! first > + > +Additional Body" && git commit --squash=first -m "Additional Body" would avoid the multi line argument > + git tag squash-with-body && > + test_tick && > + git rebase --autosquash -i HEAD^^^ && > + git log --oneline >actual && > + git log --oneline --format="%s%n%b" >actual-full && git log --format=%B ? > + test_line_count = 3 actual && > + git diff --exit-code squash-with-body && > + test 1 = "$(git cat-file blob HEAD^:file1)" && > + test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) && > + test 0 = $(grep squash actual-full | wc -l) && grep -v squash actual-full is simpler I think Best Wishes Phillip > + test 1 = $(grep Additional actual-full | wc -l) > +' > + > test_expect_success 'misspelled auto squash' ' > git reset --hard base && > echo 1 >file1 && > @@ -114,7 +134,7 @@ test_expect_success 'auto squash that matches 2 commits' ' > test_line_count = 4 actual && > git diff --exit-code final-multisquash && > test 1 = "$(git cat-file blob HEAD^^:file1)" && > - test 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) && > + test 1 = $(git cat-file commit HEAD^^ | grep first | wc -l) && > test 1 = $(git cat-file commit HEAD | grep first | wc -l) > ' > > @@ -152,7 +172,7 @@ test_expect_success 'auto squash that matches a sha1' ' > test_line_count = 3 actual && > git diff --exit-code final-shasquash && > test 1 = "$(git cat-file blob HEAD^:file1)" && > - test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l) > + test 0 = $(git cat-file commit HEAD^ | grep squash | wc -l) > ' > > test_expect_success 'auto squash that matches longer sha1' ' > @@ -168,7 +188,7 @@ test_expect_success 'auto squash that matches longer sha1' ' > test_line_count = 3 actual && > git diff --exit-code final-longshasquash && > test 1 = "$(git cat-file blob HEAD^:file1)" && > - test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l) > + test 0 = $(git cat-file commit HEAD^ | grep squash | wc -l) > ' > > test_auto_commit_flags () { > @@ -192,7 +212,7 @@ test_expect_success 'use commit --fixup' ' > ' > > test_expect_success 'use commit --squash' ' > - test_auto_commit_flags squash 2 > + test_auto_commit_flags squash 1 > ' > > test_auto_fixup_fixup () { > @@ -228,7 +248,7 @@ test_auto_fixup_fixup () { > test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) > elif test "$1" = "squash" > then > - test 3 = $(git cat-file commit HEAD^ | grep first | wc -l) > + test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) > else > false > fi > @@ -268,7 +288,7 @@ test_expect_success C_LOCALE_OUTPUT 'autosquash with custom inst format' ' > test_line_count = 3 actual && > git diff --exit-code final-squash-instFmt && > test 1 = "$(git cat-file blob HEAD^:file1)" && > - test 2 = $(git cat-file commit HEAD^ | grep squash | wc -l) > + test 0 = $(git cat-file commit HEAD^ | grep squash | wc -l) > ' > > test_expect_success 'autosquash with empty custom instructionFormat' ' > diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh > index d277a9f4b7..bfab245eb3 100755 > --- a/t/t3900-i18n-commit.sh > +++ b/t/t3900-i18n-commit.sh > @@ -226,10 +226,6 @@ test_commit_autosquash_multi_encoding () { > git rev-list HEAD >actual && > test_line_count = 3 actual && > iconv -f $old -t UTF-8 "$TEST_DIRECTORY"/t3900/$msg >expect && > - if test $flag = squash; then > - subject="$(head -1 expect)" && > - printf "\nsquash! %s\n" "$subject" >>expect > - fi && > git cat-file commit HEAD^ >raw && > (sed "1,/^$/d" raw | iconv -f $new -t utf-8) >actual && > test_cmp expect actual >