{"thread":{"id":"47474","subject":"[PATCH 1/2] commit doc: document that -c, -C, -F and --fixup with -m error","startedAt":"2017-12-20T19:39:04Z","lastAt":"2017-12-23T12:50:00Z","messageCount":16,"participants":["Ævar Arnfjörð Bjarmason","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"335100","messageId":"20171220193845.11754-1-avarab@gmail.com","threadId":"47474","inReplyTo":null,"subject":"[PATCH 1/2] commit doc: document that -c, -C, -F and --fixup with -m error","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-20T19:38:44Z","receivedAt":"2017-12-20T19:39:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Document that providing any of -c, -C, -F and --fixup along with -m\nwill result in an error. Some variant of this has been errored about\nexplicitly since 0c091296c0 (\"git-commit: log parameter updates.\",\n2005-08-08), but the documentation was never updated to reflect this.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-commit.txt | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 8c74a2ca03..df83176314 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -144,6 +144,9 @@ OPTIONS\n \tUse the given as the commit message.\n \tIf multiple `-m` options are given, their values are\n \tconcatenated as separate paragraphs.\n++\n+Combining the `-m` option and any of `-c`, `-C`, `-F` or `--fixup`\n+will result in an error.\n \n -t ::\n --template=::\n-- \n2.15.1.424.g9478a66081\n\n"},{"id":"335101","messageId":"20171220193845.11754-2-avarab@gmail.com","threadId":"47474","inReplyTo":"20171220193845.11754-1-avarab@gmail.com","subject":"[PATCH 2/2] commit: add support for --fixup -m\"\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-20T19:38:45Z","receivedAt":"2017-12-20T19:39:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add support for supplying the -m option with --fixup. Doing so has\nerrored out ever since --fixup was introduced. Before this, the only\nway to amend the fixup message while committing was to use --edit and\namend it in the editor.\n\nThe use-case for this feature is one of:\n\n * Leaving a quick note to self when creating a --fixup commit when\n it's not self-evident why the commit should be squashed without a\n note into another one.\n\n * (Ab)using the --fixup feature to \"fix up\" commits that have already\n been pushed to a branch that doesn't allow non-fast-forwards,\n i.e. just noting \"this should have been part of that other commit\",\n and if the history ever got rewritten in the future the two should\n be combined.\n\n In such a case you might want to leave a small message,\n e.g. \"forgot this part, which broke XYZ\".\n\nWhen the --fixup option was initially added the \"Option -m cannot be\ncombined\" error was expanded from -c, -C and -F to also include\n--fixup[1]\n\nThose options could also support combining with -m, but given what\nthey do I can't think of a good use-case for doing that, so I have not\nmade the more invasive change of splitting up the logic in commit.c to\nfirst act on those, and then on -m options.\n\n1. d71b8ba7c9 (\"commit: --fixup option for use with rebase\n --autosquash\", 2010-11-02)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-commit.txt | 4 ++--\n builtin/commit.c | 8 +++++---\n t/t7500-commit.sh | 9 ++++++++-\n 3 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex df83176314..4489677fd1 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -145,8 +145,8 @@ OPTIONS\n \tIf multiple `-m` options are given, their values are\n \tconcatenated as separate paragraphs.\n +\n-Combining the `-m` option and any of `-c`, `-C`, `-F` or `--fixup`\n-will result in an error.\n+Combining the `-m` option and any of `-c`, `-C` or `-F` will result in\n+an error.\n \n -t ::\n --template=::\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8a87701414..4e68394391 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -701,7 +701,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t}\n \t}\n \n-\tif (have_option_m) {\n+\tif (have_option_m && !fixup_message) {\n \t\tstrbuf_addbuf(&sb, &message);\n \t\thook_arg1 = \"message\";\n \t} else if (logfile && !strcmp(logfile, \"-\")) {\n@@ -731,6 +731,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tctx.output_encoding = get_commit_output_encoding();\n \t\tformat_commit_message(commit, \"fixup! %s\\n\\n\",\n \t\t\t\t &sb, &ctx);\n+\t\tif (have_option_m)\n+\t\t\tstrbuf_addbuf(&sb, &message);\n \t\thook_arg1 = \"message\";\n \t} else if (!stat(git_path_merge_msg(), &statbuf)) {\n \t\t/*\n@@ -1197,8 +1199,8 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tf++;\n \tif (f > 1)\n \t\tdie(_(\"Only one of -c/-C/-F/--fixup can be used.\"));\n-\tif (have_option_m && f > 0)\n-\t\tdie((_(\"Option -m cannot be combined with -c/-C/-F/--fixup.\")));\n+\tif (have_option_m && (edit_message || use_message || logfile))\n+\t\tdie((_(\"Option -m cannot be combined with -c/-C/-F.\")));\n \tif (f || have_option_m)\n \t\ttemplate_file = NULL;\n \tif (edit_message)\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex 5739d3ed23..2d95778b74 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -272,6 +272,14 @@ test_expect_success 'commit --fixup provides correct one-line commit message' '\n \tcommit_msg_is \"fixup! target message subject line\"\n '\n \n+test_expect_success 'commit --fixup -m\"something\" -m\"extra\"' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tgit commit --fixup HEAD~1 -m\"something\" -m\"extra\" &&\n+\tcommit_msg_is \"fixup! target message subject linesomething\n+\n+extra\"\n+'\n+\n test_expect_success 'commit --squash works with -F' '\n \tcommit_for_rebase_autosquash_setup &&\n \techo \"log message from file\" >msgfile &&\n@@ -325,7 +333,6 @@ test_expect_success 'invalid message options when using --fixup' '\n \ttest_must_fail git commit --fixup HEAD~1 --squash HEAD~2 &&\n \ttest_must_fail git commit --fixup HEAD~1 -C HEAD~2 &&\n \ttest_must_fail git commit --fixup HEAD~1 -c HEAD~2 &&\n-\ttest_must_fail git commit --fixup HEAD~1 -m \"cmdline message\" &&\n \ttest_must_fail git commit --fixup HEAD~1 -F log\n '\n \n-- \n2.15.1.424.g9478a66081\n\n"},{"id":"335103","messageId":"CAPig+cR_erp_U5GJKBcoBnQ+xhWpS4KjTu3sNgRTTO4V==DCsA@mail.gmail.com","threadId":"47474","inReplyTo":"20171220193845.11754-1-avarab@gmail.com","subject":"Re: [PATCH 1/2] commit doc: document that -c, -C, -F and --fixup with -m error","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-20T19:45:30Z","receivedAt":"2017-12-20T19:45:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> Document that providing any of -c, -C, -F and --fixup along with -m\n> will result in an error. Some variant of this has been errored about\n> explicitly since 0c091296c0 (\"git-commit: log parameter updates.\",\n> 2005-08-08), but the documentation was never updated to reflect this.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> @@ -144,6 +144,9 @@ OPTIONS\n> ++\n> +Combining the `-m` option and any of `-c`, `-C`, `-F` or `--fixup`\n> +will result in an error.\n\nThis sounds a bit scary, as if there is something wrong with Git.\nPerhaps say instead that they are \"mutually exclusive\":\n\n The `-m` option is mutually exclusive with `-c`, `-C`, `-F`, and\n `--fixup`.\n\nOr something.\n"},{"id":"335104","messageId":"CAPig+cSeRZR7J9WSozTETVu=Y9N2wMRpoCgUkLXyzAaBOmTNZw@mail.gmail.com","threadId":"47474","inReplyTo":"20171220193845.11754-2-avarab@gmail.com","subject":"Re: [PATCH 2/2] commit: add support for --fixup -m\"\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-20T20:03:48Z","receivedAt":"2017-12-20T20:03:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> Add support for supplying the -m option with --fixup. Doing so has\n> errored out ever since --fixup was introduced. Before this, the only\n> way to amend the fixup message while committing was to use --edit and\n> amend it in the editor.\n\nI can't tell, based upon this description, if '-m --edit' is a\nvalid combination and, if so, does it correctly open the editor after\nappending ?\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\n> @@ -272,6 +272,14 @@ test_expect_success 'commit --fixup provides correct one-line commit message' '\n> commit_msg_is \"fixup! target message subject line\"\n> '\n>\n> +test_expect_success 'commit --fixup -m\"something\" -m\"extra\"' '\n> + commit_for_rebase_autosquash_setup &&\n> + git commit --fixup HEAD~1 -m\"something\" -m\"extra\" &&\n> + commit_msg_is \"fixup! target message subject linesomething\n> +\n> +extra\"\n> +'\n\nHmm, so the first -m appended to the \"fixup!\" line, but the second -m\nappended after a blank line? That doesn't seem very intuitive.\n\nAlso, doesn't the text following \"fixup!\" need to exactly match the\nsummary line of the commit message in order for \"git rebase -i\n--autosquash\" to work? Am I overlooking something obvious?\n"},{"id":"335108","messageId":"87bmit5b8a.fsf@evledraar.gmail.com","threadId":"47474","inReplyTo":"CAPig+cSeRZR7J9WSozTETVu=Y9N2wMRpoCgUkLXyzAaBOmTNZw@mail.gmail.com","subject":"Re: [PATCH 2/2] commit: add support for --fixup -m\"\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-20T21:40:53Z","receivedAt":"2017-12-20T21:41:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Dec 20 2017, Eric Sunshine jotted:\n\n> On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason\n> wrote:\n>> Add support for supplying the -m option with --fixup. Doing so has\n>> errored out ever since --fixup was introduced. Before this, the only\n>> way to amend the fixup message while committing was to use --edit and\n>> amend it in the editor.\n>\n> I can't tell, based upon this description, if '-m --edit' is a\n> valid combination and, if so, does it correctly open the editor after\n> appending ?\n\nYes, that's works as expected for all the options, and this doesn't\nbreak that.\n\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\n>> @@ -272,6 +272,14 @@ test_expect_success 'commit --fixup provides correct one-line commit message' '\n>> commit_msg_is \"fixup! target message subject line\"\n>> '\n>>\n>> +test_expect_success 'commit --fixup -m\"something\" -m\"extra\"' '\n>> + commit_for_rebase_autosquash_setup &&\n>> + git commit --fixup HEAD~1 -m\"something\" -m\"extra\" &&\n>> + commit_msg_is \"fixup! target message subject linesomething\n>> +\n>> +extra\"\n>> +'\n>\n> Hmm, so the first -m appended to the \"fixup!\" line, but the second -m\n> appended after a blank line? That doesn't seem very intuitive.\n>\n> Also, doesn't the text following \"fixup!\" need to exactly match the\n> summary line of the commit message in order for \"git rebase -i\n> --autosquash\" to work? Am I overlooking something obvious?\n\nIt does the right thing and it's actually\n\"$fixup_line\\n\\n$first_m\\n\\n$second_m\" etc. It's just that this\ncommit_msg_is function is testing against the \"%s%b\" format, so the\nfirst line of the body comes right after the subject.\n\nThanks for the review of both patches. I'll clarify the points raised in\ncommit message for v2 pending further feedback.\n"},{"id":"335109","messageId":"CAPig+cRDi14Rez+6ZJAEdxHpo0Mw7JQFkcuG2oVqv2pP_ZbvhQ@mail.gmail.com","threadId":"47474","inReplyTo":"87bmit5b8a.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/2] commit: add support for --fixup -m\"\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-20T21:50:45Z","receivedAt":"2017-12-20T21:50:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 20, 2017 at 4:40 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> On Wed, Dec 20 2017, Eric Sunshine jotted:\n>> On Wed, Dec 20, 2017 at 2:38 PM, Ævar Arnfjörð Bjarmason\n>> wrote:\n>>> +test_expect_success 'commit --fixup -m\"something\" -m\"extra\"' '\n>>> + commit_for_rebase_autosquash_setup &&\n>>> + git commit --fixup HEAD~1 -m\"something\" -m\"extra\" &&\n>>> + commit_msg_is \"fixup! target message subject linesomething\n>>> +\n>>> +extra\"\n>>> +'\n>>\n>> Hmm, so the first -m appended to the \"fixup!\" line, but the second -m\n>> appended after a blank line? That doesn't seem very intuitive.\n>>\n>> Also, doesn't the text following \"fixup!\" need to exactly match the\n>> summary line of the commit message in order for \"git rebase -i\n>> --autosquash\" to work? Am I overlooking something obvious?\n>\n> It does the right thing and it's actually\n> \"$fixup_line\\n\\n$first_m\\n\\n$second_m\" etc. It's just that this\n> commit_msg_is function is testing against the \"%s%b\" format, so the\n> first line of the body comes right after the subject.\n\nThanks for explaining. I guess I should have delved into\ncommit_msg_is() before commenting.\n"},{"id":"335171","messageId":"20171222160056.10102-1-avarab@gmail.com","threadId":"47474","inReplyTo":"CAPig+cRDi14Rez+6ZJAEdxHpo0Mw7JQFkcuG2oVqv2pP_ZbvhQ@mail.gmail.com","subject":"[PATCH v2 0/2] support -m\"\" combined with commit --fixup","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-22T16:00:54Z","receivedAt":"2017-12-22T16:01:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Here's a hopefully ready to apply v2 incorporating feedback from Eric\n(thanks!). A tbdiff with v1 follows below.\n\nÆvar Arnfjörð Bjarmason (2):\n commit doc: document that -c, -C, -F and --fixup with -m error\n commit: add support for --fixup -m\"\"\n\n Documentation/git-commit.txt | 2 ++\n builtin/commit.c | 8 +++++---\n t/t7500-commit.sh | 9 ++++++++-\n 3 files changed, 15 insertions(+), 4 deletions(-)\n\n1: 7d5e2531ee ! 1: 82333992ec commit doc: document that -c, -C, -F and --fixup with -m error\n @@ -7,6 +7,7 @@\n explicitly since 0c091296c0 (\"git-commit: log parameter updates.\",\n 2005-08-08), but the documentation was never updated to reflect this.\n \n + Wording-by: Eric Sunshine \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n @@ -17,8 +18,8 @@\n \tIf multiple `-m` options are given, their values are\n \tconcatenated as separate paragraphs.\n ++\n -+Combining the `-m` option and any of `-c`, `-C`, `-F` or `--fixup`\n -+will result in an error.\n ++The `-m` option is mutually exclusive with `-c`, `-C`, `-F`, and\n ++`--fixup`.\n \n -t ::\n --template=::\n2: bd78a211ed ! 2: 780de6e042 commit: add support for --fixup -m\"\"\n @@ -22,6 +22,21 @@\n In such a case you might want to leave a small message,\n e.g. \"forgot this part, which broke XYZ\".\n \n + With this, --fixup -m\"More\" -m\"Details\" will result in a\n + commit message like:\n + \n + !fixup >\n + \n + More\n + \n + Details\n + \n + The reason the test being added here seems to squash \"More\" at the end\n + of the subject line of the commit being fixed up is because the test\n + code is using \"%s%b\" so the body immediately follows the subject, it's\n + not a bug in this code, and other tests t7500-commit.sh do the same\n + thing.\n + \n When the --fixup option was initially added the \"Option -m cannot be\n combined\" error was expanded from -c, -C and -F to also include\n --fixup[1]\n @@ -34,6 +49,7 @@\n 1. d71b8ba7c9 (\"commit: --fixup option for use with rebase\n --autosquash\", 2010-11-02)\n \n + Helped-by: Eric Sunshine \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n @@ -43,10 +59,9 @@\n \tIf multiple `-m` options are given, their values are\n \tconcatenated as separate paragraphs.\n +\n --Combining the `-m` option and any of `-c`, `-C`, `-F` or `--fixup`\n --will result in an error.\n -+Combining the `-m` option and any of `-c`, `-C` or `-F` will result in\n -+an error.\n +-The `-m` option is mutually exclusive with `-c`, `-C`, `-F`, and\n +-`--fixup`.\n ++The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n \n -t ::\n --template=::\n\n-- \n2.15.1.424.g9478a66081\n\n\n"},{"id":"335172","messageId":"20171222160056.10102-2-avarab@gmail.com","threadId":"47474","inReplyTo":"20171222160056.10102-1-avarab@gmail.com","subject":"[PATCH v2 1/2] commit doc: document that -c, -C, -F and --fixup with -m error","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-22T16:00:55Z","receivedAt":"2017-12-22T16:01:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Document that providing any of -c, -C, -F and --fixup along with -m\nwill result in an error. Some variant of this has been errored about\nexplicitly since 0c091296c0 (\"git-commit: log parameter updates.\",\n2005-08-08), but the documentation was never updated to reflect this.\n\nWording-by: Eric Sunshine \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-commit.txt | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 8c74a2ca03..3fbb7352bc 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -144,6 +144,9 @@ OPTIONS\n \tUse the given as the commit message.\n \tIf multiple `-m` options are given, their values are\n \tconcatenated as separate paragraphs.\n++\n+The `-m` option is mutually exclusive with `-c`, `-C`, `-F`, and\n+`--fixup`.\n \n -t ::\n --template=::\n-- \n2.15.1.424.g9478a66081\n\n"},{"id":"335184","messageId":"CAPig+cQj8=YFRBQZ3+9NJAsr2rNq0GvxrUpmt18ttvmVu1Nyhg@mail.gmail.com","threadId":"47474","inReplyTo":"20171222160056.10102-1-avarab@gmail.com","subject":"Re: [PATCH v2 0/2] support -m\"\" combined with commit --fixup","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-22T19:53:26Z","receivedAt":"2017-12-22T19:53:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Dec 22, 2017 at 11:00 AM, Ævar Arnfjörð Bjarmason\n wrote:\n> Here's a hopefully ready to apply v2 incorporating feedback from Eric\n> (thanks!). A tbdiff with v1 follows below.\n>\n> Ævar Arnfjörð Bjarmason (2):\n> commit doc: document that -c, -C, -F and --fixup with -m error\n> commit: add support for --fixup -m\"\"\n\nPatch 2/2 doesn't seem to have made it to the list...\n\n> 2: bd78a211ed ! 2: 780de6e042 commit: add support for --fixup -m\"\"\n> @@ -22,6 +22,21 @@\n> In such a case you might want to leave a small message,\n> e.g. \"forgot this part, which broke XYZ\".\n>\n> + With this, --fixup -m\"More\" -m\"Details\" will result in a\n> + commit message like:\n> +\n> + !fixup >\n> +\n> + More\n> +\n> + Details\n> +\n> + The reason the test being added here seems to squash \"More\" at the end\n> + of the subject line of the commit being fixed up is because the test\n> + code is using \"%s%b\" so the body immediately follows the subject, it's\n> + not a bug in this code, and other tests t7500-commit.sh do the same\n> + thing.\n\nDid you also intend to mention something about --edit still working\nwith -m? (Or do we assume that people will understand automatically\nthat it does?)\n"},{"id":"335191","messageId":"20171222204152.4822-1-avarab@gmail.com","threadId":"47474","inReplyTo":"CAPig+cRDi14Rez+6ZJAEdxHpo0Mw7JQFkcuG2oVqv2pP_ZbvhQ@mail.gmail.com","subject":"[PATCH v2 2/2] commit: add support for --fixup -m\"\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-22T20:41:52Z","receivedAt":"2017-12-22T20:42:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add support for supplying the -m option with --fixup. Doing so has\nerrored out ever since --fixup was introduced. Before this, the only\nway to amend the fixup message while committing was to use --edit and\namend it in the editor.\n\nThe use-case for this feature is one of:\n\n * Leaving a quick note to self when creating a --fixup commit when\n it's not self-evident why the commit should be squashed without a\n note into another one.\n\n * (Ab)using the --fixup feature to \"fix up\" commits that have already\n been pushed to a branch that doesn't allow non-fast-forwards,\n i.e. just noting \"this should have been part of that other commit\",\n and if the history ever got rewritten in the future the two should\n be combined.\n\n In such a case you might want to leave a small message,\n e.g. \"forgot this part, which broke XYZ\".\n\nWith this, --fixup -m\"More\" -m\"Details\" will result in a\ncommit message like:\n\n !fixup >\n\n More\n\n Details\n\nThe reason the test being added here seems to squash \"More\" at the end\nof the subject line of the commit being fixed up is because the test\ncode is using \"%s%b\" so the body immediately follows the subject, it's\nnot a bug in this code, and other tests t7500-commit.sh do the same\nthing.\n\nWhen the --fixup option was initially added the \"Option -m cannot be\ncombined\" error was expanded from -c, -C and -F to also include\n--fixup[1]\n\nThose options could also support combining with -m, but given what\nthey do I can't think of a good use-case for doing that, so I have not\nmade the more invasive change of splitting up the logic in commit.c to\nfirst act on those, and then on -m options.\n\n1. d71b8ba7c9 (\"commit: --fixup option for use with rebase\n --autosquash\", 2010-11-02)\n\nHelped-by: Eric Sunshine \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-commit.txt | 3 +--\n builtin/commit.c | 8 +++++---\n t/t7500-commit.sh | 9 ++++++++-\n 3 files changed, 14 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 3fbb7352bc..f970a43422 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -145,8 +145,7 @@ OPTIONS\n \tIf multiple `-m` options are given, their values are\n \tconcatenated as separate paragraphs.\n +\n-The `-m` option is mutually exclusive with `-c`, `-C`, `-F`, and\n-`--fixup`.\n+The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n \n -t ::\n --template=::\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8a87701414..4e68394391 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -701,7 +701,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t}\n \t}\n \n-\tif (have_option_m) {\n+\tif (have_option_m && !fixup_message) {\n \t\tstrbuf_addbuf(&sb, &message);\n \t\thook_arg1 = \"message\";\n \t} else if (logfile && !strcmp(logfile, \"-\")) {\n@@ -731,6 +731,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tctx.output_encoding = get_commit_output_encoding();\n \t\tformat_commit_message(commit, \"fixup! %s\\n\\n\",\n \t\t\t\t &sb, &ctx);\n+\t\tif (have_option_m)\n+\t\t\tstrbuf_addbuf(&sb, &message);\n \t\thook_arg1 = \"message\";\n \t} else if (!stat(git_path_merge_msg(), &statbuf)) {\n \t\t/*\n@@ -1197,8 +1199,8 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tf++;\n \tif (f > 1)\n \t\tdie(_(\"Only one of -c/-C/-F/--fixup can be used.\"));\n-\tif (have_option_m && f > 0)\n-\t\tdie((_(\"Option -m cannot be combined with -c/-C/-F/--fixup.\")));\n+\tif (have_option_m && (edit_message || use_message || logfile))\n+\t\tdie((_(\"Option -m cannot be combined with -c/-C/-F.\")));\n \tif (f || have_option_m)\n \t\ttemplate_file = NULL;\n \tif (edit_message)\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex 5739d3ed23..2d95778b74 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -272,6 +272,14 @@ test_expect_success 'commit --fixup provides correct one-line commit message' '\n \tcommit_msg_is \"fixup! target message subject line\"\n '\n \n+test_expect_success 'commit --fixup -m\"something\" -m\"extra\"' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tgit commit --fixup HEAD~1 -m\"something\" -m\"extra\" &&\n+\tcommit_msg_is \"fixup! target message subject linesomething\n+\n+extra\"\n+'\n+\n test_expect_success 'commit --squash works with -F' '\n \tcommit_for_rebase_autosquash_setup &&\n \techo \"log message from file\" >msgfile &&\n@@ -325,7 +333,6 @@ test_expect_success 'invalid message options when using --fixup' '\n \ttest_must_fail git commit --fixup HEAD~1 --squash HEAD~2 &&\n \ttest_must_fail git commit --fixup HEAD~1 -C HEAD~2 &&\n \ttest_must_fail git commit --fixup HEAD~1 -c HEAD~2 &&\n-\ttest_must_fail git commit --fixup HEAD~1 -m \"cmdline message\" &&\n \ttest_must_fail git commit --fixup HEAD~1 -F log\n '\n \n-- \n2.15.1.424.g9478a66081\n\n"},{"id":"335192","messageId":"87k1xesdd3.fsf@evledraar.gmail.com","threadId":"47474","inReplyTo":"CAPig+cQj8=YFRBQZ3+9NJAsr2rNq0GvxrUpmt18ttvmVu1Nyhg@mail.gmail.com","subject":"Re: [PATCH v2 0/2] support -m\"\" combined with commit --fixup","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-22T20:43:04Z","receivedAt":"2017-12-22T20:43:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 22 2017, Eric Sunshine jotted:\n\n> On Fri, Dec 22, 2017 at 11:00 AM, Ævar Arnfjörð Bjarmason\n> wrote:\n>> Here's a hopefully ready to apply v2 incorporating feedback from Eric\n>> (thanks!). A tbdiff with v1 follows below.\n>>\n>> Ævar Arnfjörð Bjarmason (2):\n>> commit doc: document that -c, -C, -F and --fixup with -m error\n>> commit: add support for --fixup -m\"\"\n>\n> Patch 2/2 doesn't seem to have made it to the list...\n\nOops, here it comes.\n\n>> 2: bd78a211ed ! 2: 780de6e042 commit: add support for --fixup -m\"\"\n>> @@ -22,6 +22,21 @@\n>> In such a case you might want to leave a small message,\n>> e.g. \"forgot this part, which broke XYZ\".\n>>\n>> + With this, --fixup -m\"More\" -m\"Details\" will result in a\n>> + commit message like:\n>> +\n>> + !fixup >\n>> +\n>> + More\n>> +\n>> + Details\n>> +\n>> + The reason the test being added here seems to squash \"More\" at the end\n>> + of the subject line of the commit being fixed up is because the test\n>> + code is using \"%s%b\" so the body immediately follows the subject, it's\n>> + not a bug in this code, and other tests t7500-commit.sh do the same\n>> + thing.\n>\n> Did you also intend to mention something about --edit still working\n> with -m? (Or do we assume that people will understand automatically\n> that it does?)\n\nI thought it was clear enough since it works with --fixup now and with\neverything else, and the commit message was already getting long\nenough...\n"},{"id":"335196","messageId":"xmqqzi6api5a.fsf@gitster.mtv.corp.google.com","threadId":"47474","inReplyTo":"20171222204152.4822-1-avarab@gmail.com","subject":"Re: [PATCH v2 2/2] commit: add support for --fixup -m\"\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-22T21:28:01Z","receivedAt":"2017-12-22T21:28:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Those options could also support combining with -m, but given what\n> they do I can't think of a good use-case for doing that, so I have not\n> made the more invasive change of splitting up the logic in commit.c to\n> first act on those, and then on -m options.\n>\n> 1. d71b8ba7c9 (\"commit: --fixup option for use with rebase\n> --autosquash\", 2010-11-02)\n\nTo be fair, when \"rebase --autosquash -i\" is run (which is why you\nwould use --fixup in the first place), the log message of the fixup\none is used only for locating which one is to be corrected, and the\ncontents of the log message is discarded. So \"given what it does\",\nI can't think of a good use-case for using --fixup and -m together,\neither. So \"nobody immediately thought of it when it was added\" is\ncertainly not a reason to later make the combination possible.\n\nBut I personally am moderately negative on one of these two imagined\nuse cases.\n\n> Add support for supplying the -m option with --fixup. Doing so has\n> errored out ever since --fixup was introduced. Before this, the only\n> way to amend the fixup message while committing was to use --edit and\n> amend it in the editor.\n>\n> The use-case for this feature is one of:\n>\n> * Leaving a quick note to self when creating a --fixup commit when\n> it's not self-evident why the commit should be squashed without a\n> note into another one.\n\nThis is probably OK.\n\n> * (Ab)using the --fixup feature to \"fix up\" commits that have already\n> been pushed to a branch that doesn't allow non-fast-forwards,\n> i.e. just noting \"this should have been part of that other commit\",\n> and if the history ever got rewritten in the future the two should\n> be combined.\n\nThis has a smell of the tail wagging the dog.\n\nPerhaps your editor does not have a good integration with external\ncommands to allow you to insert a single-liner output from\n\n git show --date=short -s --pretty='format:%h (\"%s\", %ad)' \"$1\"\n\nand that is what you are abusing --fixup for? \n\nIt is simply bad practice to leave a log entry that begins with\n!fixup marker that would confuse automated tools like \"rebase -i\"\nmachinery on a commit that you have no intention of squashing into\nanother, as it invites mistakes. \n\nI do agree with the scenario where you would wish you could take\nback an earlier mistake but you cannot. But the log for such a\nfollow-up fix should be written just like any other follow-up fix\ncommit, i.e. describe what was wrong and how the wrongness is\ncorrected with the follow-up change. What was wrong in \"which\ncommit\" is of course important part, but it is a relatively small\npart.\n"},{"id":"335197","messageId":"xmqqvagypi35.fsf@gitster.mtv.corp.google.com","threadId":"47474","inReplyTo":"xmqqzi6api5a.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] commit: add support for --fixup -m\"\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-22T21:29:18Z","receivedAt":"2017-12-22T21:29:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> ... So \"nobody immediately thought of it when it was added\" is\n> certainly not a reason to later make the combination possible.\n\nOops, not a reason to later \"_not_ to\" make an update, of course.\n\n"},{"id":"335200","messageId":"87h8sis9w5.fsf@evledraar.gmail.com","threadId":"47474","inReplyTo":"xmqqzi6api5a.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] commit: add support for --fixup -m\"\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-22T21:58:02Z","receivedAt":"2017-12-22T21:58:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 22 2017, Junio C. Hamano jotted:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Those options could also support combining with -m, but given what\n>> they do I can't think of a good use-case for doing that, so I have not\n>> made the more invasive change of splitting up the logic in commit.c to\n>> first act on those, and then on -m options.\n>>\n>> 1. d71b8ba7c9 (\"commit: --fixup option for use with rebase\n>> --autosquash\", 2010-11-02)\n>\n> To be fair, when \"rebase --autosquash -i\" is run (which is why you\n> would use --fixup in the first place), the log message of the fixup\n> one is used only for locating which one is to be corrected, and the\n> contents of the log message is discarded. So \"given what it does\",\n> I can't think of a good use-case for using --fixup and -m together,\n> either. So \"nobody immediately thought of it when it was added\" is\n> certainly not a reason to later make the combination possible.\n>\n> But I personally am moderately negative on one of these two imagined\n> use cases.\n>\n>> Add support for supplying the -m option with --fixup. Doing so has\n>> errored out ever since --fixup was introduced. Before this, the only\n>> way to amend the fixup message while committing was to use --edit and\n>> amend it in the editor.\n>>\n>> The use-case for this feature is one of:\n>>\n>> * Leaving a quick note to self when creating a --fixup commit when\n>> it's not self-evident why the commit should be squashed without a\n>> note into another one.\n>\n> This is probably OK.\n>\n>> * (Ab)using the --fixup feature to \"fix up\" commits that have already\n>> been pushed to a branch that doesn't allow non-fast-forwards,\n>> i.e. just noting \"this should have been part of that other commit\",\n>> and if the history ever got rewritten in the future the two should\n>> be combined.\n>\n> This has a smell of the tail wagging the dog.\n>\n> Perhaps your editor does not have a good integration with external\n> commands to allow you to insert a single-liner output from\n>\n> git show --date=short -s --pretty='format:%h (\"%s\", %ad)' \"$1\"\n\nSince this use-case is talking about pushing a fixup for an already\npushed commit, this is the first thing I put in -m\"...\" to reduce\nambiguity.\n\n> and that is what you are abusing --fixup for?\n>\n> It is simply bad practice to leave a log entry that begins with\n> !fixup marker that would confuse automated tools like \"rebase -i\"\n> machinery on a commit that you have no intention of squashing into\n> another, as it invites mistakes.\n\n \"if the history ever got rewritten in the future the two should be\n combined\"\n\nSo it's still the intent to squash these, it's just not being done right\nnow, and even if it never happens it'll be easy to glance at the\nrelevant commits in log --oneline.\n\n> I do agree with the scenario where you would wish you could take\n> back an earlier mistake but you cannot. But the log for such a\n> follow-up fix should be written just like any other follow-up fix\n> commit, i.e. describe what was wrong and how the wrongness is\n> corrected with the follow-up change. What was wrong in \"which\n> commit\" is of course important part, but it is a relatively small\n> part.\n\nI don't agree that git as a tool should be so opinionated. You can edit\nthese --fixup messages right now with --edit, and I do. That it doesn't\nwork with -m\"\" as it should is a longstanding UI wart.\n\nTools should be naturally composable without needless arbitrary\nlimitations.\n"},{"id":"335201","messageId":"xmqqfu82pgdg.fsf@gitster.mtv.corp.google.com","threadId":"47474","inReplyTo":"87h8sis9w5.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 2/2] commit: add support for --fixup -m\"\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-22T22:06:19Z","receivedAt":"2017-12-22T22:06:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I don't agree that git as a tool should be so opinionated. You can edit\n> these --fixup messages right now with --edit, and I do. That it doesn't\n> work with -m\"\" as it should is a longstanding UI wart.\n\nI think you missed the point.\n\nI was expressing my opinion, not an opinion of Git as a tool, that I\nthink one of these two \"use case\" scenario was a bad way not to be\nencouraged.\n\nThat is totally different from allowing --fixup and -m working\ntogether. That is a good thing that helps the other \"good\" use case.\n"},{"id":"335222","messageId":"87a7y9sj68.fsf@evledraar.gmail.com","threadId":"47474","inReplyTo":"xmqqfu82pgdg.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] commit: add support for --fixup -m\"\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-23T12:49:51Z","receivedAt":"2017-12-23T12:50:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 22 2017, Junio C. Hamano jotted:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> I don't agree that git as a tool should be so opinionated. You can edit\n>> these --fixup messages right now with --edit, and I do. That it doesn't\n>> work with -m\"\" as it should is a longstanding UI wart.\n>\n> I think you missed the point.\n\nI did, yes. Sorry, I was being a bit touchy and argumentative there.\n\n> I was expressing my opinion, not an opinion of Git as a tool, that I\n> think one of these two \"use case\" scenario was a bad way not to be\n> encouraged.\n>\n> That is totally different from allowing --fixup and -m working\n> together. That is a good thing that helps the other \"good\" use case.\n"}]}