{"thread":{"id":"52574","subject":"[PATCH 0/1] sequencer: comment out the 'squash!' line","startedAt":"2020-01-06T16:04:14Z","lastAt":"2020-01-08T16:53:11Z","messageCount":15,"participants":["Michael Rappazzo via GitGitGadget","Phillip Wood","Junio C Hamano","Mike Rappazzo","Jeff King","brian m. carlson","Jonathan Nieder","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"389264","messageId":"pull.511.git.1578326648.gitgitgadget@gmail.com","threadId":"52574","inReplyTo":null,"subject":"[PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Michael Rappazzo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-06T16:04:07Z","receivedAt":"2020-01-06T16:04:14Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"When performing a squash commit, the commit comments are combined into a\nsingle commit. Since the subject line of the squash commit is used to\nidentify the squash-to target commit, it cannot offer any useful\ncontribution to the new commit message. Therefore, the squash commit subject\nline it commented out from the combined message (much like a fixup commit's\nfull comment).\n\nThe body of a squash commit may contain additional content to add to the\ncommit message, so this part of the squash commit message is retained.\n\nSince this change what the expected post-rebase commit comment would look\nlike, related test expectations are adjusted to reflect the the new\nexpectation. A new test is added for the new expectation.\n\nSigned-off-by: Michael Rappazzo rappazzo@gmail.com [rappazzo@gmail.com]\n\nMichael Rappazzo (1):\n  sequencer: comment out the 'squash!' line\n\n sequencer.c                   |  1 +\n t/t3404-rebase-interactive.sh |  4 +---\n t/t3415-rebase-autosquash.sh  | 36 +++++++++++++++++++++++++++--------\n t/t3900-i18n-commit.sh        |  4 ----\n 4 files changed, 30 insertions(+), 15 deletions(-)\n\n\nbase-commit: 8679ef24ed64018bb62170c43ce73e0261c0600a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-511%2Frappazzo%2Fcomment-squash-subject-line-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-511/rappazzo/comment-squash-subject-line-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/511\n-- \ngitgitgadget\n"},{"id":"389265","messageId":"b262a9d099b882339e9cb930b0a09fd5fe6734b0.1578326648.git.gitgitgadget@gmail.com","threadId":"52574","inReplyTo":"pull.511.git.1578326648.gitgitgadget@gmail.com","subject":"[PATCH 1/1] sequencer: comment out the 'squash!' line","fromName":"Michael Rappazzo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-06T16:04:08Z","receivedAt":"2020-01-06T16:04:15Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"From: Michael Rappazzo <rappazzo@gmail.com>\n\nWhen performing a squash commit, the commit comments are combined into a\nsingle commit.  Since the subject line of the squash commit is used to\nidentify the squash-to target commit, it cannot offer any useful\ncontribution to the new commit message.  Therefore, the squash commit\nsubject line it commented out from the combined message (much like a\nfixup commit's full comment).\n\nThe body of a squash commit may contain additional content to add to the\ncommit message, so this part of the squash commit message is retained.\n\nSince this change what the expected post-rebase commit comment would look\nlike, related test expectations are adjusted to reflect the the new\nexpectation.  A new test is added for the new expectation.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n sequencer.c                   |  1 +\n t/t3404-rebase-interactive.sh |  4 +---\n t/t3415-rebase-autosquash.sh  | 36 +++++++++++++++++++++++++++--------\n t/t3900-i18n-commit.sh        |  4 ----\n 4 files changed, 30 insertions(+), 15 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 763ccbbc45..e5602686d7 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1756,6 +1756,7 @@ static int update_squash_messages(struct repository *r,\n \t\tstrbuf_addf(&buf, _(\"This is the commit message #%d:\"),\n \t\t\t    ++opts->current_fixup_count + 1);\n \t\tstrbuf_addstr(&buf, \"\\n\\n\");\n+\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n \t\tstrbuf_addstr(&buf, body);\n \t} else if (command == TODO_FIXUP) {\n \t\tstrbuf_addf(&buf, \"\\n%c \", comment_line_char);\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex ae6e55ce79..57d178d431 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -513,8 +513,6 @@ test_expect_success C_LOCALE_OUTPUT 'squash and fixup generate correct log messa\n \tcat >expect-squash-fixup <<-\\EOF &&\n \tB\n \n-\tD\n-\n \tONCE\n \tEOF\n \tgit checkout -b squash-fixup E &&\n@@ -1325,7 +1323,7 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)'\n \ttest_cmp_rev HEAD F &&\n \trm file6 &&\n \tgit rebase --continue &&\n-\ttest $(git cat-file commit HEAD | sed -ne \\$p) = I &&\n+\ttest $(git cat-file commit HEAD | sed -ne \\$p) = F &&\n \tgit reset --hard original-branch2\n '\n \ndiff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\nindex 22d218698e..51c5a94aea 100755\n--- a/t/t3415-rebase-autosquash.sh\n+++ b/t/t3415-rebase-autosquash.sh\n@@ -59,7 +59,6 @@ test_auto_squash () {\n \tgit add -u &&\n \ttest_tick &&\n \tgit commit -m \"squash! first\" &&\n-\n \tgit tag $1 &&\n \ttest_tick &&\n \tgit rebase $2 -i HEAD^^^ &&\n@@ -67,7 +66,7 @@ test_auto_squash () {\n \ttest_line_count = 3 actual &&\n \tgit diff --exit-code $1 &&\n \ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n-\ttest 2 = $(git cat-file commit HEAD^ | grep first | wc -l)\n+\ttest 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n }\n \n test_expect_success 'auto squash (option)' '\n@@ -82,6 +81,27 @@ test_expect_success 'auto squash (config)' '\n \ttest_must_fail test_auto_squash final-squash-config-false\n '\n \n+test_expect_success 'auto squash includes squash body but not squash directive' '\n+\tgit reset --hard base &&\n+\techo 1 >file1 &&\n+\tgit add -u &&\n+\ttest_tick &&\n+\tgit commit -m \"squash! first\n+\n+Additional Body\" &&\n+\tgit tag squash-with-body &&\n+\ttest_tick &&\n+\tgit rebase --autosquash -i HEAD^^^ &&\n+\tgit log --oneline >actual &&\n+\tgit log --oneline --format=\"%s%n%b\" >actual-full &&\n+\ttest_line_count = 3 actual &&\n+\tgit diff --exit-code squash-with-body &&\n+\ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n+\ttest 1 = $(git cat-file commit HEAD^ | grep first | wc -l) &&\n+\ttest 0 = $(grep squash actual-full | wc -l) &&\n+\ttest 1 = $(grep Additional actual-full | wc -l)\n+'\n+\n test_expect_success 'misspelled auto squash' '\n \tgit reset --hard base &&\n \techo 1 >file1 &&\n@@ -114,7 +134,7 @@ test_expect_success 'auto squash that matches 2 commits' '\n \ttest_line_count = 4 actual &&\n \tgit diff --exit-code final-multisquash &&\n \ttest 1 = \"$(git cat-file blob HEAD^^:file1)\" &&\n-\ttest 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&\n+\ttest 1 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&\n \ttest 1 = $(git cat-file commit HEAD | grep first | wc -l)\n '\n \n@@ -152,7 +172,7 @@ test_expect_success 'auto squash that matches a sha1' '\n \ttest_line_count = 3 actual &&\n \tgit diff --exit-code final-shasquash &&\n \ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n-\ttest 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n+\ttest 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n '\n \n test_expect_success 'auto squash that matches longer sha1' '\n@@ -168,7 +188,7 @@ test_expect_success 'auto squash that matches longer sha1' '\n \ttest_line_count = 3 actual &&\n \tgit diff --exit-code final-longshasquash &&\n \ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n-\ttest 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n+\ttest 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n '\n \n test_auto_commit_flags () {\n@@ -192,7 +212,7 @@ test_expect_success 'use commit --fixup' '\n '\n \n test_expect_success 'use commit --squash' '\n-\ttest_auto_commit_flags squash 2\n+\ttest_auto_commit_flags squash 1\n '\n \n test_auto_fixup_fixup () {\n@@ -228,7 +248,7 @@ test_auto_fixup_fixup () {\n \t\ttest 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n \telif test \"$1\" = \"squash\"\n \tthen\n-\t\ttest 3 = $(git cat-file commit HEAD^ | grep first | wc -l)\n+\t\ttest 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n \telse\n \t\tfalse\n \tfi\n@@ -268,7 +288,7 @@ test_expect_success C_LOCALE_OUTPUT 'autosquash with custom inst format' '\n \ttest_line_count = 3 actual &&\n \tgit diff --exit-code final-squash-instFmt &&\n \ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n-\ttest 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n+\ttest 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n '\n \n test_expect_success 'autosquash with empty custom instructionFormat' '\ndiff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\nindex d277a9f4b7..bfab245eb3 100755\n--- a/t/t3900-i18n-commit.sh\n+++ b/t/t3900-i18n-commit.sh\n@@ -226,10 +226,6 @@ test_commit_autosquash_multi_encoding () {\n \t\tgit rev-list HEAD >actual &&\n \t\ttest_line_count = 3 actual &&\n \t\ticonv -f $old -t UTF-8 \"$TEST_DIRECTORY\"/t3900/$msg >expect &&\n-\t\tif test $flag = squash; then\n-\t\t\tsubject=\"$(head -1 expect)\" &&\n-\t\t\tprintf \"\\nsquash! %s\\n\" \"$subject\" >>expect\n-\t\tfi &&\n \t\tgit cat-file commit HEAD^ >raw &&\n \t\t(sed \"1,/^$/d\" raw | iconv -f $new -t utf-8) >actual &&\n \t\ttest_cmp expect actual\n-- \ngitgitgadget\n"},{"id":"389268","messageId":"13b47c13-7a8b-a205-0cdb-5fdcb8d42412@gmail.com","threadId":"52574","inReplyTo":"b262a9d099b882339e9cb930b0a09fd5fe6734b0.1578326648.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] sequencer: comment out the 'squash!' line","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-01-06T17:10:46Z","receivedAt":"2020-01-06T17:10:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Michael\n\nOn 06/01/2020 16:04, Michael Rappazzo via GitGitGadget wrote:\n> From: Michael Rappazzo <rappazzo@gmail.com>\n> \n> When performing a squash commit, the commit comments are combined into a\n> single commit.  Since the subject line of the squash commit is used to\n> identify the squash-to target commit, it cannot offer any useful\n> contribution to the new commit message.  Therefore, the squash commit\n> subject line it commented out from the combined message (much like a\n> fixup commit's full comment).\n\nI like the idea but I think it would be better to only comment out the \nsubject of the commit message if it starts with squash! for fixup! \notherwise it may well be a useful part of the message. For correctness I \nthink it would be better to comment out the subject (everything before \nthe first blank line as returned by `git log --pretty=%s`) rather than \njust the first line. I've actually implemented this as part of a longer \nseries that I've never got round to posting to the list[1] - feel free \nto use any of that which you find useful. That commit also shows an \nalternative way to change the --autosquash tests.\n\n[1] \nhttps://github.com/phillipwood/git/commit/b91b492e4aba1ac8d244859361379d5063cfc2b8\n\n> The body of a squash commit may contain additional content to add to the\n> commit message, so this part of the squash commit message is retained.\n> \n> Since this change what the expected post-rebase commit comment would look\n> like, related test expectations are adjusted to reflect the the new\n> expectation.  A new test is added for the new expectation.\n> \n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> ---\n>   sequencer.c                   |  1 +\n>   t/t3404-rebase-interactive.sh |  4 +---\n>   t/t3415-rebase-autosquash.sh  | 36 +++++++++++++++++++++++++++--------\n>   t/t3900-i18n-commit.sh        |  4 ----\n>   4 files changed, 30 insertions(+), 15 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 763ccbbc45..e5602686d7 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -1756,6 +1756,7 @@ static int update_squash_messages(struct repository *r,\n>   \t\tstrbuf_addf(&buf, _(\"This is the commit message #%d:\"),\n>   \t\t\t    ++opts->current_fixup_count + 1);\n>   \t\tstrbuf_addstr(&buf, \"\\n\\n\");\n> +\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n>   \t\tstrbuf_addstr(&buf, body);\n>   \t} else if (command == TODO_FIXUP) {\n>   \t\tstrbuf_addf(&buf, \"\\n%c \", comment_line_char);\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index ae6e55ce79..57d178d431 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -513,8 +513,6 @@ test_expect_success C_LOCALE_OUTPUT 'squash and fixup generate correct log messa\n>   \tcat >expect-squash-fixup <<-\\EOF &&\n>   \tB\n>   \n> -\tD\n> -\n>   \tONCE\n>   \tEOF\n>   \tgit checkout -b squash-fixup E &&\n> @@ -1325,7 +1323,7 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)'\n>   \ttest_cmp_rev HEAD F &&\n>   \trm file6 &&\n>   \tgit rebase --continue &&\n> -\ttest $(git cat-file commit HEAD | sed -ne \\$p) = I &&\n> +\ttest $(git cat-file commit HEAD | sed -ne \\$p) = F &&\n>   \tgit reset --hard original-branch2\n>   '\n>   \n> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\n> index 22d218698e..51c5a94aea 100755\n> --- a/t/t3415-rebase-autosquash.sh\n> +++ b/t/t3415-rebase-autosquash.sh\n> @@ -59,7 +59,6 @@ test_auto_squash () {\n>   \tgit add -u &&\n>   \ttest_tick &&\n>   \tgit commit -m \"squash! first\" &&\n> -\n>   \tgit tag $1 &&\n>   \ttest_tick &&\n>   \tgit rebase $2 -i HEAD^^^ &&\n> @@ -67,7 +66,7 @@ test_auto_squash () {\n>   \ttest_line_count = 3 actual &&\n>   \tgit diff --exit-code $1 &&\n>   \ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> -\ttest 2 = $(git cat-file commit HEAD^ | grep first | wc -l)\n> +\ttest 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n>   }\n>   \n>   test_expect_success 'auto squash (option)' '\n> @@ -82,6 +81,27 @@ test_expect_success 'auto squash (config)' '\n>   \ttest_must_fail test_auto_squash final-squash-config-false\n>   '\n>   \n> +test_expect_success 'auto squash includes squash body but not squash directive' '\n> +\tgit reset --hard base &&\n> +\techo 1 >file1 &&\n> +\tgit add -u &&\n> +\ttest_tick &&\n> +\tgit commit -m \"squash! first\n> +\n> +Additional Body\" &&\ngit commit --squash=first -m \"Additional Body\"\nwould avoid the multi line argument\n\n> +\tgit tag squash-with-body &&\n> +\ttest_tick &&\n> +\tgit rebase --autosquash -i HEAD^^^ &&\n> +\tgit log --oneline >actual &&\n> +\tgit log --oneline --format=\"%s%n%b\" >actual-full &&\n\ngit log --format=%B ?\n\n> +\ttest_line_count = 3 actual &&\n> +\tgit diff --exit-code squash-with-body &&\n> +\ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> +\ttest 1 = $(git cat-file commit HEAD^ | grep first | wc -l) &&\n> +\ttest 0 = $(grep squash actual-full | wc -l) &&\n\ngrep -v squash actual-full\nis simpler I think\n\nBest Wishes\n\nPhillip\n\n> +\ttest 1 = $(grep Additional actual-full | wc -l)\n> +'\n> +\n>   test_expect_success 'misspelled auto squash' '\n>   \tgit reset --hard base &&\n>   \techo 1 >file1 &&\n> @@ -114,7 +134,7 @@ test_expect_success 'auto squash that matches 2 commits' '\n>   \ttest_line_count = 4 actual &&\n>   \tgit diff --exit-code final-multisquash &&\n>   \ttest 1 = \"$(git cat-file blob HEAD^^:file1)\" &&\n> -\ttest 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&\n> +\ttest 1 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&\n>   \ttest 1 = $(git cat-file commit HEAD | grep first | wc -l)\n>   '\n>   \n> @@ -152,7 +172,7 @@ test_expect_success 'auto squash that matches a sha1' '\n>   \ttest_line_count = 3 actual &&\n>   \tgit diff --exit-code final-shasquash &&\n>   \ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> -\ttest 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> +\ttest 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n>   '\n>   \n>   test_expect_success 'auto squash that matches longer sha1' '\n> @@ -168,7 +188,7 @@ test_expect_success 'auto squash that matches longer sha1' '\n>   \ttest_line_count = 3 actual &&\n>   \tgit diff --exit-code final-longshasquash &&\n>   \ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> -\ttest 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> +\ttest 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n>   '\n>   \n>   test_auto_commit_flags () {\n> @@ -192,7 +212,7 @@ test_expect_success 'use commit --fixup' '\n>   '\n>   \n>   test_expect_success 'use commit --squash' '\n> -\ttest_auto_commit_flags squash 2\n> +\ttest_auto_commit_flags squash 1\n>   '\n>   \n>   test_auto_fixup_fixup () {\n> @@ -228,7 +248,7 @@ test_auto_fixup_fixup () {\n>   \t\ttest 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n>   \telif test \"$1\" = \"squash\"\n>   \tthen\n> -\t\ttest 3 = $(git cat-file commit HEAD^ | grep first | wc -l)\n> +\t\ttest 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n>   \telse\n>   \t\tfalse\n>   \tfi\n> @@ -268,7 +288,7 @@ test_expect_success C_LOCALE_OUTPUT 'autosquash with custom inst format' '\n>   \ttest_line_count = 3 actual &&\n>   \tgit diff --exit-code final-squash-instFmt &&\n>   \ttest 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> -\ttest 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> +\ttest 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n>   '\n>   \n>   test_expect_success 'autosquash with empty custom instructionFormat' '\n> diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\n> index d277a9f4b7..bfab245eb3 100755\n> --- a/t/t3900-i18n-commit.sh\n> +++ b/t/t3900-i18n-commit.sh\n> @@ -226,10 +226,6 @@ test_commit_autosquash_multi_encoding () {\n>   \t\tgit rev-list HEAD >actual &&\n>   \t\ttest_line_count = 3 actual &&\n>   \t\ticonv -f $old -t UTF-8 \"$TEST_DIRECTORY\"/t3900/$msg >expect &&\n> -\t\tif test $flag = squash; then\n> -\t\t\tsubject=\"$(head -1 expect)\" &&\n> -\t\t\tprintf \"\\nsquash! %s\\n\" \"$subject\" >>expect\n> -\t\tfi &&\n>   \t\tgit cat-file commit HEAD^ >raw &&\n>   \t\t(sed \"1,/^$/d\" raw | iconv -f $new -t utf-8) >actual &&\n>   \t\ttest_cmp expect actual\n> \n"},{"id":"389271","messageId":"xmqq7e24a3t0.fsf@gitster-ct.c.googlers.com","threadId":"52574","inReplyTo":"pull.511.git.1578326648.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-06T17:32:43Z","receivedAt":"2020-01-06T17:32:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael Rappazzo via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> Since this change what the expected post-rebase commit comment would look\n> like, related test expectations are adjusted to reflect the the new\n> expectation. A new test is added for the new expectation.\n\nDoesn't that mean automated tools people may have written require\nsimilar adjustment to continue working correctly if this change is\napplied?\n\nCan you tell us more about your expected use case?  I am imagining\nthat most people use the log messages from both/all commits being\nsquashed when manually editing to perfect the final log message (as\nopposed to mechanically processing the concatenated message), so it\nshouldn't matter if the squash! title is untouched or commented out\nto them, and those (probably minority) who are mechanical processing\nwill be hurt with this change, so I do not quite see the point of\nthis patch.\n\nThanks.\n\n>\n> Signed-off-by: Michael Rappazzo rappazzo@gmail.com [rappazzo@gmail.com]\n>\n> Michael Rappazzo (1):\n>   sequencer: comment out the 'squash!' line\n>\n>  sequencer.c                   |  1 +\n>  t/t3404-rebase-interactive.sh |  4 +---\n>  t/t3415-rebase-autosquash.sh  | 36 +++++++++++++++++++++++++++--------\n>  t/t3900-i18n-commit.sh        |  4 ----\n>  4 files changed, 30 insertions(+), 15 deletions(-)\n>\n>\n> base-commit: 8679ef24ed64018bb62170c43ce73e0261c0600a\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-511%2Frappazzo%2Fcomment-squash-subject-line-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-511/rappazzo/comment-squash-subject-line-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/511\n"},{"id":"389272","messageId":"CANoM8SVnrRafqF4F70vFtUQpbM=u6j9MFc9XNgUvjZMz32hYbA@mail.gmail.com","threadId":"52574","inReplyTo":"13b47c13-7a8b-a205-0cdb-5fdcb8d42412@gmail.com","subject":"Re: [PATCH 1/1] sequencer: comment out the 'squash!' line","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2020-01-06T17:34:07Z","receivedAt":"2020-01-06T17:34:20Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Mon, Jan 6, 2020 at 12:10 PM Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> Hi Michael\n>\n> On 06/01/2020 16:04, Michael Rappazzo via GitGitGadget wrote:\n> > From: Michael Rappazzo <rappazzo@gmail.com>\n> >\n> > When performing a squash commit, the commit comments are combined into a\n> > single commit.  Since the subject line of the squash commit is used to\n> > identify the squash-to target commit, it cannot offer any useful\n> > contribution to the new commit message.  Therefore, the squash commit\n> > subject line it commented out from the combined message (much like a\n> > fixup commit's full comment).\n>\n> I like the idea but I think it would be better to only comment out the\n> subject of the commit message if it starts with squash! for fixup!\n> otherwise it may well be a useful part of the message. For correctness I\n> think it would be better to comment out the subject (everything before\n> the first blank line as returned by `git log --pretty=%s`) rather than\n> just the first line. I've actually implemented this as part of a longer\n> series that I've never got round to posting to the list[1] - feel free\n> to use any of that which you find useful. That commit also shows an\n> alternative way to change the --autosquash tests.\n>\n> [1]\n> https://github.com/phillipwood/git/commit/b91b492e4aba1ac8d244859361379d5063cfc2b8\n\nThat makes sense.  Since your implementation is probably better, it\nseems like the\nonly thing useful left from mine is the test that I added.  I'll look\nto resubmit the patch\nwith your commit.\n\n> > The body of a squash commit may contain additional content to add to the\n> > commit message, so this part of the squash commit message is retained.\n> >\n> > Since this change what the expected post-rebase commit comment would look\n> > like, related test expectations are adjusted to reflect the the new\n> > expectation.  A new test is added for the new expectation.\n> >\n> > Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> > ---\n> >   sequencer.c                   |  1 +\n> >   t/t3404-rebase-interactive.sh |  4 +---\n> >   t/t3415-rebase-autosquash.sh  | 36 +++++++++++++++++++++++++++--------\n> >   t/t3900-i18n-commit.sh        |  4 ----\n> >   4 files changed, 30 insertions(+), 15 deletions(-)\n> >\n> > diff --git a/sequencer.c b/sequencer.c\n> > index 763ccbbc45..e5602686d7 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -1756,6 +1756,7 @@ static int update_squash_messages(struct repository *r,\n> >               strbuf_addf(&buf, _(\"This is the commit message #%d:\"),\n> >                           ++opts->current_fixup_count + 1);\n> >               strbuf_addstr(&buf, \"\\n\\n\");\n> > +             strbuf_addf(&buf, \"%c \", comment_line_char);\n> >               strbuf_addstr(&buf, body);\n> >       } else if (command == TODO_FIXUP) {\n> >               strbuf_addf(&buf, \"\\n%c \", comment_line_char);\n> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> > index ae6e55ce79..57d178d431 100755\n> > --- a/t/t3404-rebase-interactive.sh\n> > +++ b/t/t3404-rebase-interactive.sh\n> > @@ -513,8 +513,6 @@ test_expect_success C_LOCALE_OUTPUT 'squash and fixup generate correct log messa\n> >       cat >expect-squash-fixup <<-\\EOF &&\n> >       B\n> >\n> > -     D\n> > -\n> >       ONCE\n> >       EOF\n> >       git checkout -b squash-fixup E &&\n> > @@ -1325,7 +1323,7 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)'\n> >       test_cmp_rev HEAD F &&\n> >       rm file6 &&\n> >       git rebase --continue &&\n> > -     test $(git cat-file commit HEAD | sed -ne \\$p) = I &&\n> > +     test $(git cat-file commit HEAD | sed -ne \\$p) = F &&\n> >       git reset --hard original-branch2\n> >   '\n> >\n> > diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\n> > index 22d218698e..51c5a94aea 100755\n> > --- a/t/t3415-rebase-autosquash.sh\n> > +++ b/t/t3415-rebase-autosquash.sh\n> > @@ -59,7 +59,6 @@ test_auto_squash () {\n> >       git add -u &&\n> >       test_tick &&\n> >       git commit -m \"squash! first\" &&\n> > -\n> >       git tag $1 &&\n> >       test_tick &&\n> >       git rebase $2 -i HEAD^^^ &&\n> > @@ -67,7 +66,7 @@ test_auto_squash () {\n> >       test_line_count = 3 actual &&\n> >       git diff --exit-code $1 &&\n> >       test 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> > -     test 2 = $(git cat-file commit HEAD^ | grep first | wc -l)\n> > +     test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n> >   }\n> >\n> >   test_expect_success 'auto squash (option)' '\n> > @@ -82,6 +81,27 @@ test_expect_success 'auto squash (config)' '\n> >       test_must_fail test_auto_squash final-squash-config-false\n> >   '\n> >\n> > +test_expect_success 'auto squash includes squash body but not squash directive' '\n> > +     git reset --hard base &&\n> > +     echo 1 >file1 &&\n> > +     git add -u &&\n> > +     test_tick &&\n> > +     git commit -m \"squash! first\n> > +\n> > +Additional Body\" &&\n> git commit --squash=first -m \"Additional Body\"\n> would avoid the multi line argument\n>\n> > +     git tag squash-with-body &&\n> > +     test_tick &&\n> > +     git rebase --autosquash -i HEAD^^^ &&\n> > +     git log --oneline >actual &&\n> > +     git log --oneline --format=\"%s%n%b\" >actual-full &&\n>\n> git log --format=%B ?\n\nThe difference is that %B has the extra blank line.  The check below wouldn't\nsee the difference, so I guess %B is easier to read.\n\n>\n> > +     test_line_count = 3 actual &&\n> > +     git diff --exit-code squash-with-body &&\n> > +     test 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> > +     test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) &&\n> > +     test 0 = $(grep squash actual-full | wc -l) &&\n>\n> grep -v squash actual-full\n> is simpler I think\n>\n> Best Wishes\n>\n> Phillip\n>\n> > +     test 1 = $(grep Additional actual-full | wc -l)\n> > +'\n> > +\n> >   test_expect_success 'misspelled auto squash' '\n> >       git reset --hard base &&\n> >       echo 1 >file1 &&\n> > @@ -114,7 +134,7 @@ test_expect_success 'auto squash that matches 2 commits' '\n> >       test_line_count = 4 actual &&\n> >       git diff --exit-code final-multisquash &&\n> >       test 1 = \"$(git cat-file blob HEAD^^:file1)\" &&\n> > -     test 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&\n> > +     test 1 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&\n> >       test 1 = $(git cat-file commit HEAD | grep first | wc -l)\n> >   '\n> >\n> > @@ -152,7 +172,7 @@ test_expect_success 'auto squash that matches a sha1' '\n> >       test_line_count = 3 actual &&\n> >       git diff --exit-code final-shasquash &&\n> >       test 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> > -     test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> > +     test 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> >   '\n> >\n> >   test_expect_success 'auto squash that matches longer sha1' '\n> > @@ -168,7 +188,7 @@ test_expect_success 'auto squash that matches longer sha1' '\n> >       test_line_count = 3 actual &&\n> >       git diff --exit-code final-longshasquash &&\n> >       test 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> > -     test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> > +     test 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> >   '\n> >\n> >   test_auto_commit_flags () {\n> > @@ -192,7 +212,7 @@ test_expect_success 'use commit --fixup' '\n> >   '\n> >\n> >   test_expect_success 'use commit --squash' '\n> > -     test_auto_commit_flags squash 2\n> > +     test_auto_commit_flags squash 1\n> >   '\n> >\n> >   test_auto_fixup_fixup () {\n> > @@ -228,7 +248,7 @@ test_auto_fixup_fixup () {\n> >               test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n> >       elif test \"$1\" = \"squash\"\n> >       then\n> > -             test 3 = $(git cat-file commit HEAD^ | grep first | wc -l)\n> > +             test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)\n> >       else\n> >               false\n> >       fi\n> > @@ -268,7 +288,7 @@ test_expect_success C_LOCALE_OUTPUT 'autosquash with custom inst format' '\n> >       test_line_count = 3 actual &&\n> >       git diff --exit-code final-squash-instFmt &&\n> >       test 1 = \"$(git cat-file blob HEAD^:file1)\" &&\n> > -     test 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> > +     test 0 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n> >   '\n> >\n> >   test_expect_success 'autosquash with empty custom instructionFormat' '\n> > diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\n> > index d277a9f4b7..bfab245eb3 100755\n> > --- a/t/t3900-i18n-commit.sh\n> > +++ b/t/t3900-i18n-commit.sh\n> > @@ -226,10 +226,6 @@ test_commit_autosquash_multi_encoding () {\n> >               git rev-list HEAD >actual &&\n> >               test_line_count = 3 actual &&\n> >               iconv -f $old -t UTF-8 \"$TEST_DIRECTORY\"/t3900/$msg >expect &&\n> > -             if test $flag = squash; then\n> > -                     subject=\"$(head -1 expect)\" &&\n> > -                     printf \"\\nsquash! %s\\n\" \"$subject\" >>expect\n> > -             fi &&\n> >               git cat-file commit HEAD^ >raw &&\n> >               (sed \"1,/^$/d\" raw | iconv -f $new -t utf-8) >actual &&\n> >               test_cmp expect actual\n> >\n"},{"id":"389277","messageId":"CANoM8SV=pT3sFrfnEqWc2xBn_c2rES0qSMsdptF0DgcxgYL94w@mail.gmail.com","threadId":"52574","inReplyTo":"xmqq7e24a3t0.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2020-01-06T19:20:09Z","receivedAt":"2020-01-06T19:20:23Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Mon, Jan 6, 2020 at 12:34 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"Michael Rappazzo via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > Since this change what the expected post-rebase commit comment would look\n> > like, related test expectations are adjusted to reflect the the new\n> > expectation. A new test is added for the new expectation.\n>\n> Doesn't that mean automated tools people may have written require\n> similar adjustment to continue working correctly if this change is\n> applied?\n>\n> Can you tell us more about your expected use case?  I am imagining\n> that most people use the log messages from both/all commits being\n> squashed when manually editing to perfect the final log message (as\n> opposed to mechanically processing the concatenated message), so it\n> shouldn't matter if the squash! title is untouched or commented out\n> to them, and those (probably minority) who are mechanical processing\n> will be hurt with this change, so I do not quite see the point of\n> this patch.\n\nThis change isn't removing the subject line from the commit message\nduring the edit phase, it is only commenting it out.  With the subject being\ncommented out, it can minimize the effort to edit during the squash.\n\nFurthermore, it can help to eliminate accidental inclusion in the final\nmessage.  Ultimately, the accidental inclusion is my motivation for\nsubmitting this.\n"},{"id":"389278","messageId":"20200106193253.GA971477@coredump.intra.peff.net","threadId":"52574","inReplyTo":"CANoM8SV=pT3sFrfnEqWc2xBn_c2rES0qSMsdptF0DgcxgYL94w@mail.gmail.com","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-06T19:32:53Z","receivedAt":"2020-01-06T19:32:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 06, 2020 at 02:20:09PM -0500, Mike Rappazzo wrote:\n\n> > Can you tell us more about your expected use case?  I am imagining\n> > that most people use the log messages from both/all commits being\n> > squashed when manually editing to perfect the final log message (as\n> > opposed to mechanically processing the concatenated message), so it\n> > shouldn't matter if the squash! title is untouched or commented out\n> > to them, and those (probably minority) who are mechanical processing\n> > will be hurt with this change, so I do not quite see the point of\n> > this patch.\n> \n> This change isn't removing the subject line from the commit message\n> during the edit phase, it is only commenting it out.  With the subject being\n> commented out, it can minimize the effort to edit during the squash.\n> \n> Furthermore, it can help to eliminate accidental inclusion in the final\n> message.  Ultimately, the accidental inclusion is my motivation for\n> submitting this.\n\nBut I thought that was the point of \"squash\" versus \"fixup\"? One\nincludes the commit message, and the other does not.\n\nI do think \"commit --squash\" is mostly useless for that reason, and I\nsuspect we could do a better job in the documentation about pushing\npeople to \"--fixup\".\n\nBut --squash _can_ be useful with other options to populate the commit\nmessage (e.g., \"--edit\", which just pre-populates the subject with the\nright \"squash!\" line but lets you otherwise write a normal commit\nmessage). If that's the workflow you're using, then I'm sympathetic to\nauto-removing just a \"squash!\" line, as it's automated garbage that is\nonly meant as a signal for --autosquash.\n\n-Peff\n"},{"id":"389284","messageId":"xmqqimlo8ghi.fsf@gitster-ct.c.googlers.com","threadId":"52574","inReplyTo":"CANoM8SV=pT3sFrfnEqWc2xBn_c2rES0qSMsdptF0DgcxgYL94w@mail.gmail.com","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-06T20:41:45Z","receivedAt":"2020-01-06T20:41:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Rappazzo <rappazzo@gmail.com> writes:\n\n> This change isn't removing the subject line from the commit message\n> during the edit phase, it is only commenting it out.  With the subject being\n> commented out, it can minimize the effort to edit during the squash.\n\nWhich means existing automation will be broken if they are not\ntaught to be aware that these subject lines can now be commented out\nif their Git is recent enough, which does not sound like a good thing.\n\n> Furthermore, it can help to eliminate accidental inclusion in the final\n> message.  Ultimately, the accidental inclusion is my motivation for\n> submitting this.\n\nYes, but that is why the concatenated messages are given to the\neditor to be \"edited\" by you, to be better than just a\nconcatenation, right?  When I deal with a \"squash\" (not \"fixup\"),\nthe end result would have a log message for a single commit that\ndescribes the single thing it does, which would not resemble to the\noriginal of any of the squashed message---and removing extra title\nlines would be the smallest part of such an edit.  So...\n"},{"id":"389308","messageId":"20200107013401.GI6570@camp.crustytoothpaste.net","threadId":"52574","inReplyTo":"CANoM8SV=pT3sFrfnEqWc2xBn_c2rES0qSMsdptF0DgcxgYL94w@mail.gmail.com","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-01-07T01:34:01Z","receivedAt":"2020-01-07T01:34:09Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-01-06 at 19:20:09, Mike Rappazzo wrote:\n> On Mon, Jan 6, 2020 at 12:34 PM Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > \"Michael Rappazzo via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> >\n> > > Since this change what the expected post-rebase commit comment would look\n> > > like, related test expectations are adjusted to reflect the the new\n> > > expectation. A new test is added for the new expectation.\n> >\n> > Doesn't that mean automated tools people may have written require\n> > similar adjustment to continue working correctly if this change is\n> > applied?\n> >\n> > Can you tell us more about your expected use case?  I am imagining\n> > that most people use the log messages from both/all commits being\n> > squashed when manually editing to perfect the final log message (as\n> > opposed to mechanically processing the concatenated message), so it\n> > shouldn't matter if the squash! title is untouched or commented out\n> > to them, and those (probably minority) who are mechanical processing\n> > will be hurt with this change, so I do not quite see the point of\n> > this patch.\n> \n> This change isn't removing the subject line from the commit message\n> during the edit phase, it is only commenting it out.  With the subject being\n> commented out, it can minimize the effort to edit during the squash.\n> \n> Furthermore, it can help to eliminate accidental inclusion in the final\n> message.  Ultimately, the accidental inclusion is my motivation for\n> submitting this.\n\nI think this series would be useful.  I've occasionally included the\n\"squash!\" line in my commit even after I've edited the rest of the\ncommit message.  It's not super frequent, but it is a hassle to have to\ndelete it, and it does happen occasionally.  Usually I catch it before I\nsend out the series for review.\n\nI can see the argument that this makes it a little harder for mechanical\nprocessing across versions, but I suspect most of that looks something\nlike \"sed -i -e '/^squash! /d' COMMIT_EDITMSG\" and it probably won't be\naffected.  We do make occasional slightly incompatible changes across\nversions in order to improve user experience, and I think a lot of folks\nwho use squash commits will find this a pleasant improvement.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"389315","messageId":"20200107033639.GH92456@google.com","threadId":"52574","inReplyTo":"20200106193253.GA971477@coredump.intra.peff.net","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2020-01-07T03:36:39Z","receivedAt":"2020-01-07T03:36:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> But I thought that was the point of \"squash\" versus \"fixup\"? One\n> includes the commit message, and the other does not.\n>\n> I do think \"commit --squash\" is mostly useless for that reason, and I\n> suspect we could do a better job in the documentation about pushing\n> people to \"--fixup\".\n>\n> But --squash _can_ be useful with other options to populate the commit\n> message (e.g., \"--edit\", which just pre-populates the subject with the\n> right \"squash!\" line but lets you otherwise write a normal commit\n> message). If that's the workflow you're using, then I'm sympathetic to\n> auto-removing just a \"squash!\" line, as it's automated garbage that is\n> only meant as a signal for --autosquash.\n\nIt's a signal for --autosquash and it gives a visual signal to humans\nof where the squashed commit came from.\n\n--squash already implies --edit, supporting this kind of workflow.\n\nIf we could turn back time and start over, would we want something\nlike the following?\n\n 1. if someone leaves the squash! message as is, include it as is in\n    the commit message without commenting out\n\n 2. if someone edits the squash! commit message to include a body\n    describing what is being squashed in, include the squash! line as\n    part of the commented marker\n\n 3. if someone leaves the (uncommented) squash! message in after being\n    presented with an editor at --autosquash time, reopen the editor\n    with some text verifying they really meant to do that\n\nIt's rare that concatenated commit messages make sense to be used as\nis, especially when trailers (sign-offs, Fixes, etc) are involved.  I\nsuspect that (3) is more important than (2) here --- we're using the\nsame space in the editor for input and output, and the result is a\nkind of error-prone process of getting the output right.\n\nSince we can't turn back time, one possibility would be to make tools\nlike \"git show --check\" notice the squash! lines.  Would that be\nuseful?\n\nOne nice thing about (2) is that it's unlikely to affect scripted use.\nThoughts?\n\nThanks,\nJonathan\n"},{"id":"389351","messageId":"20200107111556.GC1073219@coredump.intra.peff.net","threadId":"52574","inReplyTo":"20200107033639.GH92456@google.com","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-07T11:15:56Z","receivedAt":"2020-01-07T11:15:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 06, 2020 at 07:36:39PM -0800, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > But I thought that was the point of \"squash\" versus \"fixup\"? One\n> > includes the commit message, and the other does not.\n> >\n> > I do think \"commit --squash\" is mostly useless for that reason, and I\n> > suspect we could do a better job in the documentation about pushing\n> > people to \"--fixup\".\n> >\n> > But --squash _can_ be useful with other options to populate the commit\n> > message (e.g., \"--edit\", which just pre-populates the subject with the\n> > right \"squash!\" line but lets you otherwise write a normal commit\n> > message). If that's the workflow you're using, then I'm sympathetic to\n> > auto-removing just a \"squash!\" line, as it's automated garbage that is\n> > only meant as a signal for --autosquash.\n> \n> It's a signal for --autosquash and it gives a visual signal to humans\n> of where the squashed commit came from.\n\nTrue, but I think any proposal here would continue to include that text\nin the human-readable output (I was sloppy to say \"auto-remove\"; it is\nreally \"auto-comment\").\n\nOr do you mean that it's useful in the final, squashed commit? I'd argue\nthat a normal subject line might be so, but the \"squash!\" line doesn't\nsaying anything that's not in the main subject already. It tells you\nthat there _was_ a squash, but isn't erasing that origin kind of the\npoint of a squash?\n\n> --squash already implies --edit, supporting this kind of workflow.\n\nAh, that makes sense. I don't use it myself, so I did a quick test\nearlier. But I jumped too quickly to assuming I needed \"--edit\" (the\n\"--squash\" entry in git-commit(1) talks about being able to use \"-m\",\nwhich I read too much into).\n\n> If we could turn back time and start over, would we want something\n> like the following?\n> \n>  1. if someone leaves the squash! message as is, include it as is in\n>     the commit message without commenting out\n> \n>  2. if someone edits the squash! commit message to include a body\n>     describing what is being squashed in, include the squash! line as\n>     part of the commented marker\n> \n>  3. if someone leaves the (uncommented) squash! message in after being\n>     presented with an editor at --autosquash time, reopen the editor\n>     with some text verifying they really meant to do that\n> \n> It's rare that concatenated commit messages make sense to be used as\n> is, especially when trailers (sign-offs, Fixes, etc) are involved.  I\n> suspect that (3) is more important than (2) here --- we're using the\n> same space in the editor for input and output, and the result is a\n> kind of error-prone process of getting the output right.\n> \n> Since we can't turn back time, one possibility would be to make tools\n> like \"git show --check\" notice the squash! lines.  Would that be\n> useful?\n\nWhat if (3) issued a warning to stderr insted of re-invoking the editor?\nThen \"git commit --amend\" could be used to fix it, with no change in\nbehavior.\n\n-Peff\n"},{"id":"389391","messageId":"xmqqlfqj6y5n.fsf@gitster-ct.c.googlers.com","threadId":"52574","inReplyTo":"20200107013401.GI6570@camp.crustytoothpaste.net","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-07T16:15:16Z","receivedAt":"2020-01-07T16:15:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> I can see the argument that this makes it a little harder for mechanical\n> processing across versions, but I suspect most of that looks something\n> like \"sed -i -e '/^squash! /d' COMMIT_EDITMSG\" and it probably won't be\n> affected.\n\nWith the left-anchoring, the search pattern will no longer find that\nline if \"squash!\" is commented out, but people tend to be sloppy and\ndo not anchor the pattern would not notice the difference.  Perhaps\nthe downside may not be too severe?  I dunno.\n\n> We do make occasional slightly incompatible changes across\n> versions in order to improve user experience, and I think a lot of folks\n> who use squash commits will find this a pleasant improvement.\n\n"},{"id":"389445","messageId":"20200108025509.GM6570@camp.crustytoothpaste.net","threadId":"52574","inReplyTo":"xmqqlfqj6y5n.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-01-08T02:55:09Z","receivedAt":"2020-01-08T02:55:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-01-07 at 16:15:16, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> > I can see the argument that this makes it a little harder for mechanical\n> > processing across versions, but I suspect most of that looks something\n> > like \"sed -i -e '/^squash! /d' COMMIT_EDITMSG\" and it probably won't be\n> > affected.\n> \n> With the left-anchoring, the search pattern will no longer find that\n> line if \"squash!\" is commented out, but people tend to be sloppy and\n> do not anchor the pattern would not notice the difference.  Perhaps\n> the downside may not be too severe?  I dunno.\n\nSorry, I was perhaps bad at explaining this.  In my example, it would no\nlonger remove that line, but the user wouldn't care, because it would be\ncommented out and removed automatically.  So while the code wouldn't\nwork, what the user wanted would be done by Git automatically.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"389472","messageId":"nycvar.QRO.7.76.6.2001081423180.46@tvgsbejvaqbjf.bet","threadId":"52574","inReplyTo":"20200108025509.GM6570@camp.crustytoothpaste.net","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-01-08T13:23:32Z","receivedAt":"2020-01-08T13:23:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi brian,\n\nOn Wed, 8 Jan 2020, brian m. carlson wrote:\n\n> On 2020-01-07 at 16:15:16, Junio C Hamano wrote:\n> > \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> >\n> > > I can see the argument that this makes it a little harder for mechanical\n> > > processing across versions, but I suspect most of that looks something\n> > > like \"sed -i -e '/^squash! /d' COMMIT_EDITMSG\" and it probably won't be\n> > > affected.\n> >\n> > With the left-anchoring, the search pattern will no longer find that\n> > line if \"squash!\" is commented out, but people tend to be sloppy and\n> > do not anchor the pattern would not notice the difference.  Perhaps\n> > the downside may not be too severe?  I dunno.\n>\n> Sorry, I was perhaps bad at explaining this.  In my example, it would no\n> longer remove that line, but the user wouldn't care, because it would be\n> commented out and removed automatically.  So while the code wouldn't\n> work, what the user wanted would be done by Git automatically.\n\nSounds reasonable to me.\n\nCiao,\nDscho\n"},{"id":"389483","messageId":"xmqqv9pl3n65.fsf@gitster-ct.c.googlers.com","threadId":"52574","inReplyTo":"20200108025509.GM6570@camp.crustytoothpaste.net","subject":"Re: [PATCH 0/1] sequencer: comment out the 'squash!' line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-08T16:53:06Z","receivedAt":"2020-01-08T16:53:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On 2020-01-07 at 16:15:16, Junio C Hamano wrote:\n>> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n>> \n>> > I can see the argument that this makes it a little harder for mechanical\n>> > processing across versions, but I suspect most of that looks something\n>> > like \"sed -i -e '/^squash! /d' COMMIT_EDITMSG\" and it probably won't be\n>> > affected.\n>> \n>> With the left-anchoring, the search pattern will no longer find that\n>> line if \"squash!\" is commented out, but people tend to be sloppy and\n>> do not anchor the pattern would not notice the difference.  Perhaps\n>> the downside may not be too severe?  I dunno.\n>\n> Sorry, I was perhaps bad at explaining this.  In my example, it would no\n> longer remove that line, but the user wouldn't care, because it would be\n> commented out and removed automatically.  So while the code wouldn't\n> work, what the user wanted would be done by Git automatically.\n\nI didn't realize that you only care about 'd' there; you're right of\ncourse if you limit the scope of the discussion that way.\n\nI was talking in a more general terms where \"^squash!\" is used as a\nmarker that signals the boundary between two original commits and\nthe processing is done possibly differently on each part.\n\n"}]}