{"thread":{"id":"54267","subject":"[PATCH 0/3] commit: add an option to reword the last commit","startedAt":"2020-09-21T13:30:58Z","lastAt":"2020-09-24T16:58:33Z","messageCount":21,"participants":["Phillip Wood via GitGitGadget","Eric Sunshine","Junio C Hamano","Christian Couder","Phillip Wood","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"406043","messageId":"pull.736.git.1600695050.gitgitgadget@gmail.com","threadId":"54267","inReplyTo":null,"subject":"[PATCH 0/3] commit: add an option to reword the last commit","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-21T13:30:47Z","receivedAt":"2020-09-21T13:30:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"If one notices a typo in the last commit after starting to stage changes for\nthe next commit it is useful to be able to reword the last commit without\nchanging its contents. Currently the way to do that is by specifying --amend\n--only with no pathspec which is not that obvious to new users (so much so\nthat before beb635ca9c (\"commit: remove 'Clever' message for --only\n--amend\", 2016-12-09) commit printed a message to congratulate the user on\nfiguring out how to do it). If the last commit is empty one has to pass\n--allow-empty as well even though the contents are not being changed. These\npatches contain a couple of small clean ups for the commit documentation add\na --reword option for commit that rewords the last commit without changing\nits contents.\n\nPhillip Wood (3):\n commit docs: use backquotes when quoting options\n commit: reorder synopsis\n commit: add an option the reword HEAD\n\n Documentation/git-commit.txt | 30 +++++++++-----\n builtin/commit.c | 46 +++++++++++++++++++++-\n t/t7501-commit-basic-functionality.sh | 56 +++++++++++++++++++++++++++\n 3 files changed, 121 insertions(+), 11 deletions(-)\n\n\nbase-commit: 54e85e7af1ac9e9a92888060d6811ae767fea1bc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-736%2Fphillipwood%2Fwip%2Fcommit-reword-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-736/phillipwood/wip/commit-reword-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/736\n-- \ngitgitgadget\n"},{"id":"406044","messageId":"7f851e7c20aafdae5d5ae46ee1083b32ecc82c84.1600695050.git.gitgitgadget@gmail.com","threadId":"54267","inReplyTo":"pull.736.git.1600695050.gitgitgadget@gmail.com","subject":"[PATCH 3/3] commit: add an option the reword HEAD","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-21T13:30:50Z","receivedAt":"2020-09-21T13:30:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf one notices a typo in the last commit after starting to stage\nchanges for the next commit it is useful to be able to reword the last\ncommit without changing its contents. Currently the way to do that is\nby specifying --amend --only with no pathspec which is not that\nobvious to new users (so much so that before beb635ca9c (\"commit:\nremove 'Clever' message for --only --amend\", 2016-12-09) commit\nprinted a message to congratulate the user on figuring out how to do\nit). If the last commit is empty one has to pass --allow-empty as well\neven though the contents are not being changed. This commits adds a\n--reword option for commit that rewords the last commit without\nchanging its contents.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-commit.txt | 14 ++++++-\n builtin/commit.c | 46 +++++++++++++++++++++-\n t/t7501-commit-basic-functionality.sh | 56 +++++++++++++++++++++++++++\n 3 files changed, 113 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 9de4dc5d66..8ec87ecb6b 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -8,7 +8,7 @@ git-commit - Record changes to the repository\n SYNOPSIS\n --------\n [verse]\n-'git commit' [-a | --interactive | --patch] [--amend]\n+'git commit' [-a | --interactive | --patch] [--amend | --reword]\n \t [(-c | -C | --fixup | --squash) ] [-F | -m ]\n \t [--allow-empty] [--allow-empty-message] [--no-verify] [-e]\n \t [--reset-author] [--author=] [--date=]\n@@ -99,7 +99,7 @@ OPTIONS\n \tlinkgit:git-rebase[1] for details.\n \n --reset-author::\n-\tWhen used with `-C`/`-c`/`--amend` options, or when committing\n+\tWhen used with `-C`/`-c`/`--amend`/`--reword` options, or when committing\n \tafter a conflicting cherry-pick, declare that the authorship of\n \tthe resulting commit now belongs to the committer. This also\n \trenews the author timestamp.\n@@ -229,6 +229,16 @@ variable (see linkgit:git-config[1]).\n \tFor example, `git commit --amend --no-edit` amends a commit\n \twithout changing its commit message.\n \n+--reword::\n+\tReword the commit message of the tip of the current branch by\n+\treplacing it with a new commit. The commit contents will be\n+\tunchanged even if there are staged changes. This is equivalent\n+\tto specifying `--amend --only --allow-empty` with no paths.\n++\n+You should understand the implications of rewriting history if you\n+reword a commit that has already been published. (See the \"RECOVERING\n+FROM UPSTREAM REBASE\" section in linkgit:git-rebase[1].)\n+\n --amend::\n \tReplace the tip of the current branch by creating a new\n \tcommit. The recorded tree is prepared as usual (including\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 5d91b13a5c..f7913f771a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -107,6 +107,7 @@ static const char *author_message, *author_message_buffer;\n static char *edit_message, *use_message;\n static char *fixup_message, *squash_message;\n static int all, also, interactive, patch_interactive, only, amend, signoff;\n+static int reword;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n@@ -1152,6 +1153,41 @@ static void finalize_deferred_config(struct wt_status *s)\n \t\ts->ahead_behind_flags = AHEAD_BEHIND_FULL;\n }\n \n+static void validate_reword_options(int argc, struct commit *current_head)\n+{\n+\tif (!current_head)\n+\t\tdie(_(\"You have nothing to reword.\"));\n+\tif (whence != FROM_COMMIT) {\n+\t\tif (whence == FROM_MERGE)\n+\t\t\tdie(_(\"You are in the middle of a merge -- cannot \"\n+\t\t\t \"reword.\"));\n+\t\telse if (is_from_cherry_pick(whence))\n+\t\t\tdie(_(\"You are in the middle of a cherry-pick -- cannot\"\n+\t\t\t \" reword.\"));\n+\t\telse if (is_from_rebase(whence))\n+\t\t\tdie(_(\"You are in the middle of a rebase -- cannot \"\n+\t\t\t \"reword.\"));\n+\t}\n+\tif (amend)\n+\t\tdie(_(\"cannot combine --reword with --amend\"));\n+\tif (argc)\n+\t\tdie(_(\"cannot combine --reword with paths\"));\n+\tif (interactive)\n+\t\tdie(_(\"cannot combine --reword with --interactive\"));\n+\tif (patch_interactive)\n+\t\tdie(_(\"cannot combine --reword with --patch\"));\n+\tif (all)\n+\t\tdie(_(\"cannot combine --reword with --all\"));\n+\tif (also)\n+\t\tdie(_(\"cannot combine --reword with --include\"));\n+\tif (only)\n+\t\tdie(_(\"cannot combine --reword with --only\"));\n+\tif (!edit_flag && !force_author && !force_date && !renew_authorship &&\n+\t !use_message && !edit_message && !fixup_message &&\n+\t !squash_message && !logfile && !have_option_m && !signoff)\n+\t\tdie(_(\"cannot combine --reword with --no-edit\"));\n+}\n+\n static int parse_and_validate_options(int argc, const char *argv[],\n \t\t\t\t const struct option *options,\n \t\t\t\t const char * const usage[],\n@@ -1186,6 +1222,12 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\telse if (whence == FROM_REBASE_PICK)\n \t\t\tdie(_(\"You are in the middle of a rebase -- cannot amend.\"));\n \t}\n+\tif (reword) {\n+\t\tvalidate_reword_options(argc, current_head);\n+\t\tamend = 1;\n+\t\tonly = 1;\n+\t\tallow_empty = 1;\n+\t}\n \tif (fixup_message && squash_message)\n \t\tdie(_(\"Options --squash and --fixup cannot be used together\"));\n \tif (use_message)\n@@ -1208,7 +1250,8 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tuse_message = \"HEAD\";\n \tif (!use_message && !is_from_cherry_pick(whence) &&\n \t !is_from_rebase(whence) && renew_authorship)\n-\t\tdie(_(\"--reset-author can be used only with -C, -c or --amend.\"));\n+\t\tdie(_(\"--reset-author can be used only with -C, -c, --amend \"\n+\t\t \"or --reword.\"));\n \tif (use_message) {\n \t\tuse_message_buffer = read_commit_message(use_message);\n \t\tif (!renew_authorship) {\n@@ -1537,6 +1580,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL('z', \"null\", &s.null_termination,\n \t\t\t N_(\"terminate entries with NUL\")),\n \t\tOPT_BOOL(0, \"amend\", &amend, N_(\"amend previous commit\")),\n+\t\tOPT_BOOL(0, \"reword\", &reword, N_(\"reword the previous commit\")),\n \t\tOPT_BOOL(0, \"no-post-rewrite\", &no_post_rewrite, N_(\"bypass post-rewrite hook\")),\n \t\t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, N_(\"mode\"), N_(\"show untracked files, optional modes: all, normal, no. (Default: all)\"), PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n \t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\ndiff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh\nindex 110b4bf459..1ea65b426a 100755\n--- a/t/t7501-commit-basic-functionality.sh\n+++ b/t/t7501-commit-basic-functionality.sh\n@@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '\n \tgit commit --dry-run --short\n '\n \n+test_expect_success '--reword does not commit staged changes' '\n+\techo changed >file &&\n+\tgit add file &&\n+\tcat >expect <<-EOF &&\n+\t$(git log -1 --pretty=format:%B HEAD)\n+\n+\treworded\n+\tEOF\n+\tGIT_EDITOR=\"printf reworded >>\" git commit --reword &&\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp_rev HEAD@{1}^{tree} HEAD^{tree} &&\n+\ttest_cmp_rev HEAD@{1}^ HEAD^ &&\n+\tgit cat-file blob :file >actual &&\n+\ttest_cmp file actual\n+'\n+\n+test_reword_opt () {\n+\ttest_expect_success C_LOCALE_OUTPUT \"--reword incompatible with $1\" \"\n+\t\techo 'fatal: cannot combine --reword with $1' >expect &&\n+\t\ttest_must_fail git commit --reword $1 2>actual &&\n+\t\ttest_cmp expect actual\n+\t\"\n+}\n+\n+for opt in --all --amend --include --interactive --only --patch --no-edit\n+do\n+\ttest_reword_opt $opt\n+done\n+\n+test_expect_success C_LOCALE_OUTPUT '--reword with paths' '\n+\techo \"fatal: cannot combine --reword with paths\" >expect &&\n+\ttest_must_fail git commit --reword file 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_reword_no_edit () {\n+\ttest_expect_success \"--reword $@ --no-edit\" '\n+\t\tgit commit --reword '\"$@\"' --no-edit\n+\t'\n+}\n+\n+for opt in -mmessage -CHEAD^ -cHEAD --reset-author \\\n+\t\t\"--author=\\\"Commit Author \\\"\" \\\n+\t\t--date=yesterday --fixup=HEAD^ --squash=HEAD^ --signoff\n+do\n+\ttest_reword_no_edit \"$opt\"\n+done\n+\n+test_expect_success '--reword -F' '\n+\techo reworded >msg &&\n+\tgit commit --reword -F msg --no-edit &&\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp msg actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"406045","messageId":"c75566ce322435f3cf3dd67ab08b7cfa83164901.1600695050.git.gitgitgadget@gmail.com","threadId":"54267","inReplyTo":"pull.736.git.1600695050.gitgitgadget@gmail.com","subject":"[PATCH 1/3] commit docs: use backquotes when quoting options","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-21T13:30:48Z","receivedAt":"2020-09-21T13:31:01Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nMake sure that option names are typeset in a monospace font.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-commit.txt | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex a3baea32ae..a1d1d1246f 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -82,7 +82,7 @@ OPTIONS\n \n -c ::\n --reedit-message=::\n-\tLike '-C', but with `-c` the editor is invoked, so that\n+\tLike `-C`, but with `-c` the editor is invoked, so that\n \tthe user can further edit the commit message.\n \n --fixup=::\n@@ -99,10 +99,10 @@ OPTIONS\n \tlinkgit:git-rebase[1] for details.\n \n --reset-author::\n-\tWhen used with -C/-c/--amend options, or when committing after a\n-\tconflicting cherry-pick, declare that the authorship of the\n-\tresulting commit now belongs to the committer. This also renews\n-\tthe author timestamp.\n+\tWhen used with `-C`/`-c`/`--amend` options, or when committing\n+\tafter a conflicting cherry-pick, declare that the authorship of\n+\tthe resulting commit now belongs to the committer. This also\n+\trenews the author timestamp.\n \n --short::\n \tWhen doing a dry-run, give the output in the short-format. See\n-- \ngitgitgadget\n\n"},{"id":"406046","messageId":"80a69318b57348c59e45c0f8df1c342ef84388d4.1600695050.git.gitgitgadget@gmail.com","threadId":"54267","inReplyTo":"pull.736.git.1600695050.gitgitgadget@gmail.com","subject":"[PATCH 2/3] commit: reorder synopsis","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-21T13:30:49Z","receivedAt":"2020-09-21T13:31:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nRearrange the synopsis for `git commit` to try and put options that\nare used more often nearer the beginning.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-commit.txt | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex a1d1d1246f..9de4dc5d66 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -8,11 +8,11 @@ git-commit - Record changes to the repository\n SYNOPSIS\n --------\n [verse]\n-'git commit' [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]\n-\t [--dry-run] [(-c | -C | --fixup | --squash) ]\n-\t [-F | -m ] [--reset-author] [--allow-empty]\n-\t [--allow-empty-message] [--no-verify] [-e] [--author=]\n-\t [--date=] [--cleanup=] [--[no-]status]\n+'git commit' [-a | --interactive | --patch] [--amend]\n+\t [(-c | -C | --fixup | --squash) ] [-F | -m ]\n+\t [--allow-empty] [--allow-empty-message] [--no-verify] [-e]\n+\t [--reset-author] [--author=] [--date=]\n+\t [--cleanup=] [-v] [-u] [--dry-run] [--[no-]status]\n \t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n \t [-S[]] [--] [...]\n \n-- \ngitgitgadget\n\n"},{"id":"406050","messageId":"CAPig+cQNiB8nw-JwwFweHzeFYcozGA06pPKr=0N9Metp8PBbSQ@mail.gmail.com","threadId":"54267","inReplyTo":"7f851e7c20aafdae5d5ae46ee1083b32ecc82c84.1600695050.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-21T15:43:22Z","receivedAt":"2020-09-21T15:43:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Sep 21, 2020 at 9:31 AM Phillip Wood via GitGitGadget\n wrote:\n> +--reword::\n> + Reword the commit message of the tip of the current branch by\n> + replacing it with a new commit. The commit contents will be\n> + unchanged even if there are staged changes. This is equivalent\n> + to specifying `--amend --only --allow-empty` with no paths.\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> @@ -1152,6 +1153,41 @@ static void finalize_deferred_config(struct wt_status *s)\n> +static void validate_reword_options(int argc, struct commit *current_head)\n> +{\n> + if (amend)\n> + die(_(\"cannot combine --reword with --amend\"));\n> + if (only)\n> + die(_(\"cannot combine --reword with --only\"));\n\nNit: It feels a bit odd (though not outright wrong) to disallow\n--reword in combination with --amend and --only after the\ndocumentation states that --reword is equivalent to using those\noptions.\n\n> diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh\n> @@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '\n> +test_reword_opt () {\n> + test_expect_success C_LOCALE_OUTPUT \"--reword incompatible with $1\" \"\n> + echo 'fatal: cannot combine --reword with $1' >expect &&\n> + test_must_fail git commit --reword $1 2>actual &&\n> + test_cmp expect actual\n> + \"\n> +}\n\nThese error messages are subject to localization, so you'd want to use\ntest_i18ncmp() here, I think.\n\nSame comment for other new tests.\n"},{"id":"406054","messageId":"xmqqr1qvf6mg.fsf@gitster.c.googlers.com","threadId":"54267","inReplyTo":"pull.736.git.1600695050.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] commit: add an option to reword the last commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-21T16:15:51Z","receivedAt":"2020-09-21T16:16:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Phillip Wood via GitGitGadget\" writes:\n\n> If one notices a typo in the last commit after starting to stage changes for\n> the next commit it is useful to be able to reword the last commit without\n> changing its contents. Currently the way to do that is by specifying --amend\n> --only with no pathspec which is not that obvious to new users (so much so\n> that before beb635ca9c (\"commit: remove 'Clever' message for --only\n> --amend\", 2016-12-09) commit printed a message to congratulate the user on\n> figuring out how to do it).\n\n;-)\n\n\"git commit --only --amend\" could already be read as an instruction\nthat \"I am only amending without changing anything else\", but I\nagree that the new --reword verb is a good addition.\n\nThe only comment I had on the series was that the mutual-exclusivity\nlogic looked a bit fragile against feature evolution of the command,\nbut I have no concrete suggestion to make it more robust against\ntime.\n\n"},{"id":"406056","messageId":"CAP8UFD1i-zOPx6xrEFUVH1of8hzdHxO_jTOZ3fEjSXa_TONdvw@mail.gmail.com","threadId":"54267","inReplyTo":"7f851e7c20aafdae5d5ae46ee1083b32ecc82c84.1600695050.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2020-09-21T17:04:54Z","receivedAt":"2020-09-21T17:05:34Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Sep 21, 2020 at 3:33 PM Phillip Wood via GitGitGadget\n wrote:\n\n> If one notices a typo in the last commit ...\n\nIn the title \"commit: add an option the reword HEAD\", I would suggest\ns/the/to/ :-)\n"},{"id":"406059","messageId":"e9c18655-2618-90af-6d84-16ae9f575f87@gmail.com","threadId":"54267","inReplyTo":"CAP8UFD1i-zOPx6xrEFUVH1of8hzdHxO_jTOZ3fEjSXa_TONdvw@mail.gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-21T18:01:23Z","receivedAt":"2020-09-21T18:01:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 21/09/2020 18:04, Christian Couder wrote:\n> On Mon, Sep 21, 2020 at 3:33 PM Phillip Wood via GitGitGadget\n> wrote:\n> \n>> If one notices a typo in the last commit ...\n> \n> In the title \"commit: add an option the reword HEAD\", I would suggest\n> s/the/to/ :-)\n\nWell spotted Christian, Thanks\n\nPhillip\n"},{"id":"406060","messageId":"6da3752f-5e45-1381-b54c-64a81d642b72@gmail.com","threadId":"54267","inReplyTo":"CAPig+cQNiB8nw-JwwFweHzeFYcozGA06pPKr=0N9Metp8PBbSQ@mail.gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-21T18:05:08Z","receivedAt":"2020-09-21T18:05:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Eric\n\nOn 21/09/2020 16:43, Eric Sunshine wrote:\n> On Mon, Sep 21, 2020 at 9:31 AM Phillip Wood via GitGitGadget\n> wrote:\n>> +--reword::\n>> + Reword the commit message of the tip of the current branch by\n>> + replacing it with a new commit. The commit contents will be\n>> + unchanged even if there are staged changes. This is equivalent\n>> + to specifying `--amend --only --allow-empty` with no paths.\n>> diff --git a/builtin/commit.c b/builtin/commit.c\n>> @@ -1152,6 +1153,41 @@ static void finalize_deferred_config(struct wt_status *s)\n>> +static void validate_reword_options(int argc, struct commit *current_head)\n>> +{\n>> + if (amend)\n>> + die(_(\"cannot combine --reword with --amend\"));\n>> + if (only)\n>> + die(_(\"cannot combine --reword with --only\"));\n> \n> Nit: It feels a bit odd (though not outright wrong) to disallow\n> --reword in combination with --amend and --only after the\n> documentation states that --reword is equivalent to using those\n> options.\n\nYeah I decided to be quite strict, I'm in two minds about the \ndocumentation, I think it might be better to remove that line.\n\n>> diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh\n>> @@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '\n>> +test_reword_opt () {\n>> + test_expect_success C_LOCALE_OUTPUT \"--reword incompatible with $1\" \"\n>> + echo 'fatal: cannot combine --reword with $1' >expect &&\n>> + test_must_fail git commit --reword $1 2>actual &&\n>> + test_cmp expect actual\n>> + \"\n>> +}\n> \n> These error messages are subject to localization, so you'd want to use\n> test_i18ncmp() here, I think.\n> \n> Same comment for other new tests.\n\nI decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather \nthan grep so I could check the exact output. I'm slightly suspicious of \ntests that just grep for an error message when that is all we should be \nshowing. I should probably check that nothing is printed to stdout in \nthese tests\n\nBest Wishes\n\nPhillip\n\n"},{"id":"406062","messageId":"CAPig+cRdKbBZEan756gpLXBsmZCgwA+XyjUQYOWLDBTTbawGAA@mail.gmail.com","threadId":"54267","inReplyTo":"6da3752f-5e45-1381-b54c-64a81d642b72@gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-21T18:12:20Z","receivedAt":"2020-09-21T18:12:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Sep 21, 2020 at 2:05 PM Phillip Wood wrote:\n> On 21/09/2020 16:43, Eric Sunshine wrote:\n> > Nit: It feels a bit odd (though not outright wrong) to disallow\n> > --reword in combination with --amend and --only after the\n> > documentation states that --reword is equivalent to using those\n> > options.\n>\n> Yeah I decided to be quite strict, I'm in two minds about the\n> documentation, I think it might be better to remove that line.\n\nI rather like that the documentation explains the equivalency between\n--reword and `--amend --only --allow-empty` since it makes --reword\nless magical and provides the readers with pointers for further study.\n\n> I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather\n> than grep so I could check the exact output. [...]\n\nI overlooked C_LOCALE_OUTPUT.\n"},{"id":"406066","messageId":"xmqqeemvexr1.fsf@gitster.c.googlers.com","threadId":"54267","inReplyTo":"6da3752f-5e45-1381-b54c-64a81d642b72@gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-21T19:27:30Z","receivedAt":"2020-09-21T19:27:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>>> diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh\n>>> @@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '\n>>> +test_reword_opt () {\n>>> + test_expect_success C_LOCALE_OUTPUT \"--reword incompatible with $1\" \"\n>>> + echo 'fatal: cannot combine --reword with $1' >expect &&\n>>> + test_must_fail git commit --reword $1 2>actual &&\n>>> + test_cmp expect actual\n>>> + \"\n>>> +}\n>> These error messages are subject to localization, so you'd want to\n>> use\n>> test_i18ncmp() here, I think.\n>> Same comment for other new tests.\n>\n> I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather\n> than grep so I could check the exact output.\n\nI do not think it is a good idea. Dropping the C_LOCALE_OUTPUT\nprerequisite and using test_i18ncmpw would be more appropriate.\n\nA test run without GIT_TEST_GETTEXT_POISON will do the byte-for-byte\ncomparison like test_cmp. It is only the poison test, whose purpose\nis to catch commands that by mistake translated their messages, that\nwould want to mark a test that checks end-user facing messages like\nthis one as special with test_i18ncmp.\n\n> ... I should probably check that nothing is printed to stdout in\n> these tests\n\nPerhaps, but that is not the point of \"do we diagnose options thare\nare incompatble with --reword?\" test.\n\n"},{"id":"406111","messageId":"xmqqft7acre2.fsf@gitster.c.googlers.com","threadId":"54267","inReplyTo":"80a69318b57348c59e45c0f8df1c342ef84388d4.1600695050.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] commit: reorder synopsis","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-22T05:27:49Z","receivedAt":"2020-09-22T05:27:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Phillip Wood via GitGitGadget\" writes:\n\n> From: Phillip Wood \n>\n> Rearrange the synopsis for `git commit` to try and put options that\n> are used more often nearer the beginning.\n\nWhich option is used \"more often\" depends heavily on the workflow,\nand it is a bit unfriendly to those who are affected by this change\nnot to list the ones you are demoting, with rationale for each,\nsomething like:\n\n - \"--dry-run\" may or may not be used often, but it is orthogonal to\n the regular options that affect how the command behaves, and it\n is much less important and interesting than the other options.\n\nAmong the ones you are demoting without explanation\n\n - \"-s\" is no longer even mentioned, which I do not think is a well\n thought out change.\n\n - \"-v\" is to make the command do the same thing as it does without\n it, but just more loudly, so it is less interesting than the\n other options.\n\n - \"-u\" affects what is shown in the editor to prepare the log\n message, which is of less importance than other options that\n affect the outcome of the command. When --no-status is in\n effect, -u would not even have any effect, so it probably\n makes sense to move this next to \"--[no-]status\".\n\n - \"--reset-author\" can be used not just to take ownership of the\n commit from somebody else, but also be used to update the author\n date of your own change. It probably belongs to the same group\n as -c/-C/-F/-m that affects the metadata of the resulting commit\n and should stay at the original location in the list. If you\n want to change anything around this area, I'd suggest moving\n --date= close to this \"options that affect commit's\n metadata\" group.\n\n\n\n> Signed-off-by: Phillip Wood \n> ---\n> Documentation/git-commit.txt | 10 +++++-----\n> 1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index a1d1d1246f..9de4dc5d66 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -8,11 +8,11 @@ git-commit - Record changes to the repository\n> SYNOPSIS\n> --------\n> [verse]\n> -'git commit' [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]\n> -\t [--dry-run] [(-c | -C | --fixup | --squash) ]\n> -\t [-F | -m ] [--reset-author] [--allow-empty]\n> -\t [--allow-empty-message] [--no-verify] [-e] [--author=]\n> -\t [--date=] [--cleanup=] [--[no-]status]\n> +'git commit' [-a | --interactive | --patch] [--amend]\n> +\t [(-c | -C | --fixup | --squash) ] [-F | -m ]\n> +\t [--allow-empty] [--allow-empty-message] [--no-verify] [-e]\n> +\t [--reset-author] [--author=] [--date=]\n> +\t [--cleanup=] [-v] [-u] [--dry-run] [--[no-]status]\n> \t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n> \t [-S[]] [--] [...]\n"},{"id":"406125","messageId":"2a14d3d2-743c-215e-aa69-de75a576f685@gmail.com","threadId":"54267","inReplyTo":"xmqqft7acre2.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/3] commit: reorder synopsis","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-22T13:27:22Z","receivedAt":"2020-09-22T13:27:27Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 22/09/2020 06:27, Junio C Hamano wrote:\n> \"Phillip Wood via GitGitGadget\" writes:\n> \n>> From: Phillip Wood \n>>\n>> Rearrange the synopsis for `git commit` to try and put options that\n>> are used more often nearer the beginning.\n> \n> Which option is used \"more often\" depends heavily on the workflow,\n> and it is a bit unfriendly to those who are affected by this change\n> not to list the ones you are demoting, with rationale for each,\n> something like:\n\nI agree the changes are a bit subjective, I thought --dry-run -u\nand -v were probably not as important enough to justify their position\non the first line and it sounds like you basically agree with that.\n\n> - \"--dry-run\" may or may not be used often, but it is orthogonal to\n> the regular options that affect how the command behaves, and it\n> is much less important and interesting than the other options.\n> \n> Among the ones you are demoting without explanation\n> \n> - \"-s\" is no longer even mentioned, which I do not think is a well\n> thought out change.\n\nThat's a mistake I did not intend to remove it, thanks for spotting that\n \n> - \"-v\" is to make the command do the same thing as it does without\n> it, but just more loudly, so it is less interesting than the\n> other options.\n> \n> - \"-u\" affects what is shown in the editor to prepare the log\n> message, which is of less importance than other options that\n> affect the outcome of the command. When --no-status is in\n> effect, -u would not even have any effect, so it probably\n> makes sense to move this next to \"--[no-]status\".\n> \n> - \"--reset-author\" can be used not just to take ownership of the\n> commit from somebody else, but also be used to update the author\n> date of your own change. It probably belongs to the same group\n> as -c/-C/-F/-m that affects the metadata of the resulting commit\n> and should stay at the original location in the list. If you\n> want to change anything around this area, I'd suggest moving\n> --date= close to this \"options that affect commit's\n> metadata\" group.\n\nI was trying to group the author/date related option together - they \nall affect the metadata of the resulting commit\n\nWhat do you think to\n\n-'git commit' [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]\n-\t [--dry-run] [(-c | -C | --fixup | --squash) ]\n-\t [-F | -m ] [--reset-author] [--allow-empty]\n-\t [--allow-empty-message] [--no-verify] [-e] [--author=]\n-\t [--date=] [--cleanup=] [--[no-]status]\n\t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n\t [-S[]] [--] [...]\n+'git commit' [-a | --interactive | --patch] [--amend]\n+\t [(-c | -C | --fixup | --squash) ] [-F | -m ]\n+\t [--reset-author] [--author=] [--date=] [-s] [-e]\n+\t [--allow-empty] [--allow-empty-message] [--no-verify]\n+\t [--cleanup=] [-u] [--[no-]status] [-v] [--dry-run]\n\t [-i | -o | -p] [--pathspec-from-file= [--pathspec-file-nul]]\n \t [-S[]] [--] [...]\n\n--dry-run and -v are demoted. -u is moved next to --[no-]status\nas they are related. -s is restored but moved and the author/date\nrelated options are moved up to join the other options that affect the\ncommit metadata. The downside is that --allow-empty and friends end up\nbeing moved down a line as a consequence of keeping all the commit\nmetadata related options together.\n\nIf you are happy I'll redo the commit message based on your suggestions\nabove\n\nThanks\n\nPhillip\n\n> \n> \n>> Signed-off-by: Phillip Wood \n>> ---\n>> Documentation/git-commit.txt | 10 +++++-----\n>> 1 file changed, 5 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n>> index a1d1d1246f..9de4dc5d66 100644\n>> --- a/Documentation/git-commit.txt\n>> +++ b/Documentation/git-commit.txt\n>> @@ -8,11 +8,11 @@ git-commit - Record changes to the repository\n>> SYNOPSIS\n>> --------\n>> [verse]\n>> -'git commit' [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]\n>> -\t [--dry-run] [(-c | -C | --fixup | --squash) ]\n>> -\t [-F | -m ] [--reset-author] [--allow-empty]\n>> -\t [--allow-empty-message] [--no-verify] [-e] [--author=]\n>> -\t [--date=] [--cleanup=] [--[no-]status]\n>> +'git commit' [-a | --interactive | --patch] [--amend]\n>> +\t [(-c | -C | --fixup | --squash) ] [-F | -m ]\n>> +\t [--allow-empty] [--allow-empty-message] [--no-verify] [-e]\n>> +\t [--reset-author] [--author=] [--date=]\n>> +\t [--cleanup=] [-v] [-u] [--dry-run] [--[no-]status]\n>> \t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n>> \t [-S[]] [--] [...]\n"},{"id":"406126","messageId":"52217e7a-5a51-48e4-5496-96ac68602200@gmail.com","threadId":"54267","inReplyTo":"xmqqeemvexr1.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-22T13:38:03Z","receivedAt":"2020-09-22T13:38:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 21/09/2020 20:27, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>>>> diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh\n>>>> @@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '\n>>>> +test_reword_opt () {\n>>>> + test_expect_success C_LOCALE_OUTPUT \"--reword incompatible with $1\" \"\n>>>> + echo 'fatal: cannot combine --reword with $1' >expect &&\n>>>> + test_must_fail git commit --reword $1 2>actual &&\n>>>> + test_cmp expect actual\n>>>> + \"\n>>>> +}\n>>> These error messages are subject to localization, so you'd want to\n>>> use\n>>> test_i18ncmp() here, I think.\n>>> Same comment for other new tests.\n>>\n>> I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather\n>> than grep so I could check the exact output.\n> \n> I do not think it is a good idea. Dropping the C_LOCALE_OUTPUT\n> prerequisite and using test_i18ncmpw would be more appropriate.\n> \n> A test run without GIT_TEST_GETTEXT_POISON will do the byte-for-byte\n> comparison like test_cmp. It is only the poison test, whose purpose\n> is to catch commands that by mistake translated their messages, that\n> would want to mark a test that checks end-user facing messages like\n> this one as special with test_i18ncmp.\n\nThanks I wasn't aware of test_i18ncmp\n\n>> ... I should probably check that nothing is printed to stdout in\n>> these tests\n> \n> Perhaps, but that is not the point of \"do we diagnose options thare\n> are incompatble with --reword?\" test.\n\nI think it depends if one views the test as checking \"do we diagnose \noptions there are incompatible with --reword?\" or \"what do we show the \nuser when there are options that are incompatible with --reword\". For \nthe former we just want to check that the correct error message is \nprinted, for the latter we want to check that only what we expect to be \nprinted is actually printed.\n\nBest Wishes\n\nPhillip\n"},{"id":"406131","messageId":"xmqqeemtlrc9.fsf@gitster.c.googlers.com","threadId":"54267","inReplyTo":"2a14d3d2-743c-215e-aa69-de75a576f685@gmail.com","subject":"Re: [PATCH 2/3] commit: reorder synopsis","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-22T16:16:22Z","receivedAt":"2020-09-22T16:16:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> -'git commit' [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]\n> -\t [--dry-run] [(-c | -C | --fixup | --squash) ]\n> -\t [-F | -m ] [--reset-author] [--allow-empty]\n> -\t [--allow-empty-message] [--no-verify] [-e] [--author=]\n> -\t [--date=] [--cleanup=] [--[no-]status]\n> \t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n> \t [-S[]] [--] [...]\n> +'git commit' [-a | --interactive | --patch] [--amend]\n> +\t [(-c | -C | --fixup | --squash) ] [-F | -m ]\n> +\t [--reset-author] [--author=] [--date=] [-s] [-e]\n> +\t [--allow-empty] [--allow-empty-message] [--no-verify]\n> +\t [--cleanup=] [-u] [--[no-]status] [-v] [--dry-run]\n> \t [-i | -o | -p] [--pathspec-from-file= [--pathspec-file-nul]]\n> \t [-S[]] [--] [...]\n\nThis seems to group them better than the original, including\n\"--allow-empty\" and its two friends that are about sanity checking.\n\nI would actually kick \"-a\" out of the \"--interactive | --patch\"\ngroup [*1*]. Unlike the two, it is not about a different operation\nmode. It is closer to how paths whose contents will participate in\nthe resulting commit are chosen, so it is much closer to the \"-i\"\nand \"-o\" family, I would think.\n\nWhat is \"git commit -p\" you have next to \"-i\" and \"-o\"? I do not\nthink it belongs there---isn't it a short form of --patch?\n\nThanks.\n\n\n[Footnote]\n\n*1* It was originally made that way because the \"add--interactive\"\nbased operation mode was incompatible with \"-a\", I think. But the\nthing is, the interactive one is incompatible with other things like\n\"-i\" and \"-o\". Use of the '|' alternative notation to express\nthings are mutually incompatible inherently does not play well with\nthe idea of showing common things early. It would allow us to group\nconceptually similar things together, though. \n\nDo we want to say \"[-a | -i | -o | --interactive | --patch]\" in the\nsame [] group and show them as alternatives? That may logically\nmake some sense, and if we were to go that route, it belongs near\nthe end of the list, close to where we show pathspec. I dunno.\n"},{"id":"406133","messageId":"xmqq5z85lpla.fsf@gitster.c.googlers.com","threadId":"54267","inReplyTo":"52217e7a-5a51-48e4-5496-96ac68602200@gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-22T16:54:09Z","receivedAt":"2020-09-22T16:54:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>>>>> +test_reword_opt () {\n>>>>> ...\n>>> ... I should probably check that nothing is printed to stdout in\n>>> these tests\n>> Perhaps, but that is not the point of \"do we diagnose options thare\n>> are incompatble with --reword?\" test.\n>\n> I think it depends if one views the test as checking \"do we diagnose\n> options there are incompatible with --reword?\" or \"what do we show the \n> user when there are options that are incompatible with --reword\". For\n> the former we just want to check that the correct error message is \n> printed, for the latter we want to check that only what we expect to\n> be printed is actually printed.\n\nI dunno. It is plausible that we will further give some advices\n(not directly related to --reword option being incompatible with\nother options) and output may evolve. I do not think we want to be\nupdating each and every test that expects exact output.\n\nIn other words, the answer to \"what do we show when --reword and\nanother incompatible option are given?\" is \"we want to point it out\nthat --reword and --amend are not to be used together\" and not \"and\nwe should not ever say anything else\".\n\nJust like back when we started making sure everybody sets user.name\nconfiguration variable (and the way to do so was to give warnings\nand advices at strategic places), output can be added to commands\nwhere authors if each individual feature would not expect. I would\nwant to see our tests prepared for such an occasion. We cannot be\nperfectly prepared, of course, but keeping the expectation focused\nenough to check what really matters for the objective at hand would\nhelp.\n\nThanks.\n\n\n"},{"id":"406194","messageId":"nycvar.QRO.7.76.6.2009231206290.5061@tvgsbejvaqbjf.bet","threadId":"54267","inReplyTo":"7f851e7c20aafdae5d5ae46ee1083b32ecc82c84.1600695050.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-09-23T10:22:57Z","receivedAt":"2020-09-23T14:25:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Mon, 21 Sep 2020, Phillip Wood via GitGitGadget wrote:\n\n> From: Phillip Wood \n>\n> If one notices a typo in the last commit after starting to stage\n> changes for the next commit it is useful to be able to reword the last\n> commit without changing its contents. Currently the way to do that is\n> by specifying --amend --only with no pathspec which is not that\n> obvious to new users (so much so that before beb635ca9c (\"commit:\n> remove 'Clever' message for --only --amend\", 2016-12-09) commit\n> printed a message to congratulate the user on figuring out how to do\n> it). If the last commit is empty one has to pass --allow-empty as well\n> even though the contents are not being changed. This commits adds a\n> --reword option for commit that rewords the last commit without\n> changing its contents.\n\nI would like to explain the idea I tried to get across when I proposed to\nimplement support for `reword!` (and `--reword`) because I feel that it\nwill change the design of this patch in a rather big way.\n\nFirst of all, let me explain the scenario in which I long for the\n`--reword` option: I maintain several patch thickets, the most obvious one\nbeing Git for Windows' patch thicket that is merge-rebased [*1*] onto\nevery new Git version.\n\nAt times, I need to adjust a commit message in that patch thicket. It\nwould be quite wasteful to perform a full merge-rebase, therefore I\ntypically call `git commit --squash -c `, copy the\noneline, paste it after the `squash!` line (surrounded by empty lines), and\nthen reword the commit message. When the next Git version comes out, I do\na merging-rebase, and when the editor pops up because of that `squash!`\noneline, I remove the now-obsolete version(s) of the commit message.\n\nObviously, I have to be careful to either also pass `--only` (which I\nsomehow managed to learn about only today) or I have to make sure that I\nhave no staged changes. In practice, I actually specify a bogus path,\nwhich has the same effect as `--only`.\n\nWhat I would actually rather have is the `--reword` option: `git commit\n--reword `. In my mind, this would _add_ a new, \"empty\" commit,\nletting me edit the commit message of the specified commit, and using that\nas commit message, prefixed with the line `reword! `.\n\nThis, in turn, would need to be accompanied by support in the interactive\nrebase, to perform the desired reword (which is admittedly quite a bit\ndifferent from what the way the todo command `reword` works).\n\nWith that in mind, I would like to caution against the design of your\ncurrent patch, because it would slam the door shut on the way I would like\n`--reword` to work.\n\nCiao,\nDscho\n\nFootnote *1*: In Git for Windows, I want to not only rebase the patches\n(so that they are as ready to be submitted to the Git mailing list as they\ncan be) but I also want the commit history to fast-forward. The strategy I\nsettled on is the \"merging rebase\": it is a rebase that starts with a fake\nmerge of the previous commit history, i.e. merging it in using `-s ours`\nso that only the commit history comes in, but not the changes. This allows\ncontributors to pull without problems, but also provides the benefits of\nhaving a rebased version of the patches. The price is a rather big commit\nhistory on top of Git's main branch, as Git for Windows' main branch\ncontains not only the newest iteration of its patches, but _all_\niterations (at least since the first merging-rebase).\n"},{"id":"406208","messageId":"41dcb8cb-8e43-04ce-2ddd-d69c765ee327@gmail.com","threadId":"54267","inReplyTo":"nycvar.QRO.7.76.6.2009231206290.5061@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-23T18:23:20Z","receivedAt":"2020-09-23T18:23:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dscho\n\nOn 23/09/2020 11:22, Johannes Schindelin wrote:\n> Hi Phillip,\n> \n> On Mon, 21 Sep 2020, Phillip Wood via GitGitGadget wrote:\n> \n>> From: Phillip Wood \n>>\n>> If one notices a typo in the last commit after starting to stage\n>> changes for the next commit it is useful to be able to reword the last\n>> commit without changing its contents. Currently the way to do that is\n>> by specifying --amend --only with no pathspec which is not that\n>> obvious to new users (so much so that before beb635ca9c (\"commit:\n>> remove 'Clever' message for --only --amend\", 2016-12-09) commit\n>> printed a message to congratulate the user on figuring out how to do\n>> it). If the last commit is empty one has to pass --allow-empty as well\n>> even though the contents are not being changed. This commits adds a\n>> --reword option for commit that rewords the last commit without\n>> changing its contents.\n> \n> I would like to explain the idea I tried to get across when I proposed to\n> implement support for `reword!` (and `--reword`) because I feel that it\n> will change the design of this patch in a rather big way.\n> \n> First of all, let me explain the scenario in which I long for the\n> `--reword` option: I maintain several patch thickets, the most obvious one\n> being Git for Windows' patch thicket that is merge-rebased [*1*] onto\n> every new Git version.\n> \n> At times, I need to adjust a commit message in that patch thicket. It\n> would be quite wasteful to perform a full merge-rebase, therefore I\n> typically call `git commit --squash -c `, copy the\n> oneline, paste it after the `squash!` line (surrounded by empty lines), and\n> then reword the commit message. When the next Git version comes out, I do\n> a merging-rebase, and when the editor pops up because of that `squash!`\n> oneline, I remove the now-obsolete version(s) of the commit message.\n> \n> Obviously, I have to be careful to either also pass `--only` (which I\n> somehow managed to learn about only today) or I have to make sure that I\n> have no staged changes. In practice, I actually specify a bogus path,\n> which has the same effect as `--only`.\n> \n> What I would actually rather have is the `--reword` option: `git commit\n> --reword `. In my mind, this would _add_ a new, \"empty\" commit,\n> letting me edit the commit message of the specified commit, and using that\n> as commit message, prefixed with the line `reword! `.\n> \n> This, in turn, would need to be accompanied by support in the interactive\n> rebase, to perform the desired reword (which is admittedly quite a bit\n> different from what the way the todo command `reword` works).\n> \n> With that in mind, I would like to caution against the design of your\n> current patch, because it would slam the door shut on the way I would like\n> `--reword` to work.\n\nI'm keen to have an easy way to reword HEAD and a way to implement your \nreword! idea.\n\nI posted a comment on your gitgitgadget issue about reword! and drop![1] \npointing to some patches[2] that implement the reword! idea as amend!. I \nthink we want to be able to fixup a commit and reword it at the same \ntime which is way I chose the name amend! rather than reword! The \nimplementation currently changes `git commit --amend` to take an \noptional commit which isn't ideal. I wonder if calling it revise! would \nbe better then we could have `git commit --reword` to reword HEAD and \n`git commit --revise ` to create a commit that will reword and \nfixup when the user runs `git rebase -i --autostash`. fold! is \nanother possibility.\n\nI don't think this patch series stops us implementing something for \nrebase but it would mean we couldn't use the name reword! unless we \nallow `git commit --reword` to take an optional commit which I'm not \nthat keen on.\n\nWhat do you think to an alternative name?\n\nBest Wishes\n\nPhillip\n\n[1] https://github.com/gitgitgadget/git/issues/259\n[2] https://github.com/phillipwood/git/commits/wip/rebase-amend\n\n> Ciao,\n> Dscho\n> \n> Footnote *1*: In Git for Windows, I want to not only rebase the patches\n> (so that they are as ready to be submitted to the Git mailing list as they\n> can be) but I also want the commit history to fast-forward. The strategy I\n> settled on is the \"merging rebase\": it is a rebase that starts with a fake\n> merge of the previous commit history, i.e. merging it in using `-s ours`\n> so that only the commit history comes in, but not the changes. This allows\n> contributors to pull without problems, but also provides the benefits of\n> having a rebased version of the patches. The price is a rather big commit\n> history on top of Git's main branch, as Git for Windows' main branch\n> contains not only the newest iteration of its patches, but _all_\n> iterations (at least since the first merging-rebase).\n> \n\n"},{"id":"406220","messageId":"nycvar.QRO.7.76.6.2009232241140.5061@tvgsbejvaqbjf.bet","threadId":"54267","inReplyTo":"41dcb8cb-8e43-04ce-2ddd-d69c765ee327@gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-09-23T20:42:56Z","receivedAt":"2020-09-23T20:43:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Wed, 23 Sep 2020, Phillip Wood wrote:\n\n> On 23/09/2020 11:22, Johannes Schindelin wrote:\n> >\n> > On Mon, 21 Sep 2020, Phillip Wood via GitGitGadget wrote:\n> >\n> > > From: Phillip Wood \n> > >\n> > > If one notices a typo in the last commit after starting to stage\n> > > changes for the next commit it is useful to be able to reword the last\n> > > commit without changing its contents. Currently the way to do that is\n> > > by specifying --amend --only with no pathspec which is not that\n> > > obvious to new users (so much so that before beb635ca9c (\"commit:\n> > > remove 'Clever' message for --only --amend\", 2016-12-09) commit\n> > > printed a message to congratulate the user on figuring out how to do\n> > > it). If the last commit is empty one has to pass --allow-empty as well\n> > > even though the contents are not being changed. This commits adds a\n> > > --reword option for commit that rewords the last commit without\n> > > changing its contents.\n> >\n> > I would like to explain the idea I tried to get across when I proposed to\n> > implement support for `reword!` (and `--reword`) because I feel that it\n> > will change the design of this patch in a rather big way.\n> >\n> > First of all, let me explain the scenario in which I long for the\n> > `--reword` option: I maintain several patch thickets, the most obvious one\n> > being Git for Windows' patch thicket that is merge-rebased [*1*] onto\n> > every new Git version.\n> >\n> > At times, I need to adjust a commit message in that patch thicket. It\n> > would be quite wasteful to perform a full merge-rebase, therefore I\n> > typically call `git commit --squash -c `, copy the\n> > oneline, paste it after the `squash!` line (surrounded by empty lines), and\n> > then reword the commit message. When the next Git version comes out, I do\n> > a merging-rebase, and when the editor pops up because of that `squash!`\n> > oneline, I remove the now-obsolete version(s) of the commit message.\n> >\n> > Obviously, I have to be careful to either also pass `--only` (which I\n> > somehow managed to learn about only today) or I have to make sure that I\n> > have no staged changes. In practice, I actually specify a bogus path,\n> > which has the same effect as `--only`.\n> >\n> > What I would actually rather have is the `--reword` option: `git commit\n> > --reword `. In my mind, this would _add_ a new, \"empty\" commit,\n> > letting me edit the commit message of the specified commit, and using that\n> > as commit message, prefixed with the line `reword! `.\n> >\n> > This, in turn, would need to be accompanied by support in the interactive\n> > rebase, to perform the desired reword (which is admittedly quite a bit\n> > different from what the way the todo command `reword` works).\n> >\n> > With that in mind, I would like to caution against the design of your\n> > current patch, because it would slam the door shut on the way I would like\n> > `--reword` to work.\n>\n> I'm keen to have an easy way to reword HEAD and a way to implement your\n> reword! idea.\n>\n> I posted a comment on your gitgitgadget issue about reword! and drop![1]\n> pointing to some patches[2] that implement the reword! idea as amend!. I think\n> we want to be able to fixup a commit and reword it at the same time which is\n> way I chose the name amend! rather than reword! The implementation currently\n> changes `git commit --amend` to take an optional commit which isn't ideal. I\n> wonder if calling it revise! would be better then we could have `git commit\n> --reword` to reword HEAD and `git commit --revise ` to create a commit\n> that will reword and fixup when the user runs `git rebase -i\n> --autostash`. fold! is another possibility.\n>\n> I don't think this patch series stops us implementing something for rebase but\n> it would mean we couldn't use the name reword! unless we allow `git commit\n> --reword` to take an optional commit which I'm not that keen on.\n>\n> What do you think to an alternative name?\n\nI am really worried that the proliferation of confusingly similar options\nwill increase Git's reputation for being awfully hard to use.\n\nCiao,\nDscho\n"},{"id":"406246","messageId":"95cc6fb2-d1bc-11de-febe-c2b5c78a6850@gmail.com","threadId":"54267","inReplyTo":"nycvar.QRO.7.76.6.2009232241140.5061@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-24T09:58:54Z","receivedAt":"2020-09-24T09:59:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dscho\n\nOn 23/09/2020 21:42, Johannes Schindelin wrote:\n> Hi Phillip,\n> \n> On Wed, 23 Sep 2020, Phillip Wood wrote:\n> \n>> On 23/09/2020 11:22, Johannes Schindelin wrote:\n>>>\n>>> On Mon, 21 Sep 2020, Phillip Wood via GitGitGadget wrote:\n>>>\n>>>> From: Phillip Wood \n>>>>\n>>>> If one notices a typo in the last commit after starting to stage\n>>>> changes for the next commit it is useful to be able to reword the last\n>>>> commit without changing its contents. Currently the way to do that is\n>>>> by specifying --amend --only with no pathspec which is not that\n>>>> obvious to new users (so much so that before beb635ca9c (\"commit:\n>>>> remove 'Clever' message for --only --amend\", 2016-12-09) commit\n>>>> printed a message to congratulate the user on figuring out how to do\n>>>> it). If the last commit is empty one has to pass --allow-empty as well\n>>>> even though the contents are not being changed. This commits adds a\n>>>> --reword option for commit that rewords the last commit without\n>>>> changing its contents.\n>>>\n>>> I would like to explain the idea I tried to get across when I proposed to\n>>> implement support for `reword!` (and `--reword`) because I feel that it\n>>> will change the design of this patch in a rather big way.\n>>>\n>>> First of all, let me explain the scenario in which I long for the\n>>> `--reword` option: I maintain several patch thickets, the most obvious one\n>>> being Git for Windows' patch thicket that is merge-rebased [*1*] onto\n>>> every new Git version.\n>>>\n>>> At times, I need to adjust a commit message in that patch thicket. It\n>>> would be quite wasteful to perform a full merge-rebase, therefore I\n>>> typically call `git commit --squash -c `, copy the\n>>> oneline, paste it after the `squash!` line (surrounded by empty lines), and\n>>> then reword the commit message. When the next Git version comes out, I do\n>>> a merging-rebase, and when the editor pops up because of that `squash!`\n>>> oneline, I remove the now-obsolete version(s) of the commit message.\n>>>\n>>> Obviously, I have to be careful to either also pass `--only` (which I\n>>> somehow managed to learn about only today) or I have to make sure that I\n>>> have no staged changes. In practice, I actually specify a bogus path,\n>>> which has the same effect as `--only`.\n>>>\n>>> What I would actually rather have is the `--reword` option: `git commit\n>>> --reword `. In my mind, this would _add_ a new, \"empty\" commit,\n>>> letting me edit the commit message of the specified commit, and using that\n>>> as commit message, prefixed with the line `reword! `.\n>>>\n>>> This, in turn, would need to be accompanied by support in the interactive\n>>> rebase, to perform the desired reword (which is admittedly quite a bit\n>>> different from what the way the todo command `reword` works).\n>>>\n>>> With that in mind, I would like to caution against the design of your\n>>> current patch, because it would slam the door shut on the way I would like\n>>> `--reword` to work.\n>>\n>> I'm keen to have an easy way to reword HEAD and a way to implement your\n>> reword! idea.\n>>\n>> I posted a comment on your gitgitgadget issue about reword! and drop![1]\n>> pointing to some patches[2] that implement the reword! idea as amend!. I think\n>> we want to be able to fixup a commit and reword it at the same time which is\n>> way I chose the name amend! rather than reword! The implementation currently\n>> changes `git commit --amend` to take an optional commit which isn't ideal. I\n>> wonder if calling it revise! would be better then we could have `git commit\n>> --reword` to reword HEAD and `git commit --revise ` to create a commit\n>> that will reword and fixup when the user runs `git rebase -i\n>> --autostash`. fold! is another possibility.\n>>\n>> I don't think this patch series stops us implementing something for rebase but\n>> it would mean we couldn't use the name reword! unless we allow `git commit\n>> --reword` to take an optional commit which I'm not that keen on.\n>>\n>> What do you think to an alternative name?\n> \n> I am really worried that the proliferation of confusingly similar options\n> will increase Git's reputation for being awfully hard to use.\n\nThat is certainly a consideration, but not having a way to easily reword \nthe last commit without changing its contents does not to improve Git's \nuser friendliness. If you only just discovered using --only for \nrewording it's a fair bet a lot of regular users are unaware of it.\n\nThe reason I'm not keen on having --amend or --reword take an optional \ncommit is that I think it is confusing as it means sometimes that option \ncreates a new commit and sometimes it modifies the last commit \nfurthermore passing --reword=HEAD would not reword HEAD but creates a \nreword! commit.\n\nRewording the last commit and creating a reword! commit are two \ndifferent operations so I'm not sure having different options for them \nis that bad. To me the real confusion is that we end up with 3 options \nthat create different flavors of fixup commits. It would be much nicer \nif there was a single fixup type that reworded the message as well as \nfixing up the contents and users just passed `--no-edit` to avoid \nchanging the message. I'd really like to somehow change the semantics of \n`git commit --fixup/--squash` and the rebase `fixup`/`squash` commands \nto actually reword the commit. I guess that would mean an opt-in config \nsetting which isn't ideal.\n\nAs an aside I'd like to see a new `rewrite` command that wraps the \nfunctionality of `rebase -i` so the user does not have to deal with \nfixups and editing a todo list. `git rewrite amend ` would be \nequivalent to starting a rebase and marking as `edit`, the user \ncan then make their changes and run `git rewrite continue` which would \nfinish the rebase or `git rewrite amend ` which would \neither alter the todo list to mark as `edit` or create \nnew entries to rewind the rebase if is not in the todo \nlist and then run `git rebase --continue`. Additionally there would be \n`reword` and `drop` commands and support for blame so that the user can \ndo all this from an editor which would run `git rewrite amend -L \n ` to amend the commit that introduced the \ncurrent line. I've got a nasty scheme prototype of this and I've found \nit really useful. For an individual developer developing a patch series \nit is much more convenient to just edit the history directly rather than \ncreating fixup! commits (which often seem to have conflicts when they \nare applied). It would not address your use where you don't want to be \nrebasing all the time though.\n\nBest Wishes\n\nPhillip\n\n> Ciao,\n> Dscho\n> \n"},{"id":"406271","messageId":"xmqqft77glhn.fsf@gitster.c.googlers.com","threadId":"54267","inReplyTo":"95cc6fb2-d1bc-11de-febe-c2b5c78a6850@gmail.com","subject":"Re: [PATCH 3/3] commit: add an option the reword HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-24T16:58:28Z","receivedAt":"2020-09-24T16:58:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>>> I don't think this patch series stops us implementing something for rebase but\n>>> it would mean we couldn't use the name reword! unless we allow `git commit\n>>> --reword` to take an optional commit which I'm not that keen on.\n>>>\n>>> What do you think to an alternative name?\n>> I am really worried that the proliferation of confusingly similar\n>> options\n>> will increase Git's reputation for being awfully hard to use.\n> ...\n> The reason I'm not keen on having --amend or --reword take an optional\n> commit is that I think it is confusing as it means sometimes that\n> option creates a new commit and sometimes it modifies the last commit \n> furthermore passing --reword=HEAD would not reword HEAD but creates a\n> reword! commit.\n\nAdding just another subjective view to the two already presented,\nbut I think --reword, as presented by Phillip, sits better next to\nthe existing --amend.\n\nI wonder if we can extend the existing \"--fixup \" (and\nperhaps \"--squash \") to make them work better with the\nworkflow Dscho envisions? Explicit presence of the \"-e\" option\nmight be a way to tell the command to behave differently from the\ncurrent \"--fixup\" and to leave a mark that is different from\n'fixup!\" in the resulting commit to affect the later \"rebase\" step\nas well, for example.\n"}]}