commit: add an option to reword the last commit

From: Phillip Wood <[email protected]>

Rearrange the synopsis for git commit to try and put options that are used more often nearer the beginning.

Signed-off-by Phillip Wood <[email protected]>

---
 Documentation/git-commit.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Documentation/git-commit.txt +5 -5

index a1d1d1246f..9de4dc5d66 100644
@@ -8,11 +8,11 @@ git-commit - Record changes to the repository
 SYNOPSIS
 --------
 [verse]
-'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]
-	   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]
-	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
-	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
-	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
+'git commit' [-a | --interactive | --patch] [--amend]
+	   [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]
+	   [--allow-empty] [--allow-empty-message] [--no-verify] [-e]
+	   [--reset-author] [--author=<author>] [--date=<date>]
+	   [--cleanup=<mode>] [-v] [-u<mode>] [--dry-run] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
 	   [-S[<keyid>]] [--] [<pathspec>...]

Quoted 4 lines · Phillip Wood via GitGitGadget

From: Phillip Wood <[email protected]>

Rearrange the synopsis for git commit to try and put options that are used more often nearer the beginning.

Which option is used "more often" depends heavily on the workflow, and it is a bit unfriendly to those who are affected by this change not to list the ones you are demoting, with rationale for each, something like:

- "--dry-run" may or may not be used often, but it is orthogonal to the regular options that affect how the command behaves, and it is much less important and interesting than the other options.

Among the ones you are demoting without explanation

- "-s" is no longer even mentioned, which I do not think is a well thought out change.

- "-v" is to make the command do the same thing as it does without it, but just more loudly, so it is less interesting than the other options.

- "-u<mode>" affects what is shown in the editor to prepare the log message, which is of less importance than other options that affect the outcome of the command. When --no-status is in effect, -u<mode> would not even have any effect, so it probably makes sense to move this next to "--[no-]status".

- "--reset-author" can be used not just to take ownership of the commit from somebody else, but also be used to update the author date of your own change. It probably belongs to the same group as -c/-C/-F/-m that affects the metadata of the resulting commit and should stay at the original location in the list. If you want to change anything around this area, I'd suggest moving --date=<date> close to this "options that affect commit's metadata" group.

Quoted 27 lines

Signed-off-by Phillip Wood <[email protected]>

---
  Documentation/git-commit.txt | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

Documentation/git-commit.txt +5 -5

index a1d1d1246f..9de4dc5d66 100644
@@ -8,11 +8,11 @@ git-commit - Record changes to the repository
  SYNOPSIS
  --------
  [verse]
-'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]
-	   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]
-	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
-	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
-	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
+'git commit' [-a | --interactive | --patch] [--amend]
+	   [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]
+	   [--allow-empty] [--allow-empty-message] [--no-verify] [-e]
+	   [--reset-author] [--author=<author>] [--date=<date>]
+	   [--cleanup=<mode>] [-v] [-u<mode>] [--dry-run] [--[no-]status]
  	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
  	   [-S[<keyid>]] [--] [<pathspec>...]

-'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]

Documentation/git-commit.txt +0 -4

-	   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]
-	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
-	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
-	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]

	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
	   [-S[<keyid>]] [--] [<pathspec>...]

+'git commit' [-a | --interactive | --patch] [--amend]

(quoted diff) +4 -0

+	   [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]
+	   [--reset-author] [--author=<author>] [--date=<date>] [-s] [-e]
+	   [--allow-empty] [--allow-empty-message] [--no-verify]
+	   [--cleanup=<mode>] [-u<mode>] [--[no-]status]  [-v] [--dry-run]

	   [-i | -o | -p] [--pathspec-from-file=<file> [--pathspec-file-nul]]

[-S[<keyid>]] [--] [<pathspec>...]

Quoted 10 lines · Phillip Wood via GitGitGadget

+--reword::

Documentation/git-commit.txt +4 -0

+       Reword the commit message of the tip of the current branch by
+       replacing it with a new commit. The commit contents will be
+       unchanged even if there are staged changes. This is equivalent
+       to specifying `--amend --only --allow-empty` with no paths.

builtin/commit.c +3 -0

@@ -1152,6 +1153,41 @@ static void finalize_deferred_config(struct wt_status *s)
+static void validate_reword_options(int argc, struct commit *current_head)
+{
+       if (amend)

builtin/commit.c static void finalize_deferred_config(struct wt_status *s) · line 1184

Quoted 3 lines

builtin/commit.c +3 -0

+               die(_("cannot combine --reword with --amend"));
+       if (only)
+               die(_("cannot combine --reword with --only"));

Nit: It feels a bit odd (though not outright wrong) to disallow --reword in combination with --amend and --only after the documentation states that --reword is equivalent to using those options.

t/t7501-commit-basic-functionality.sh test_expect_success '--dry-run --short' ' · line 719

Quoted 9 lines

t/t7501-commit-basic-functionality.sh +7 -0

@@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '
+test_reword_opt () {
+       test_expect_success C_LOCALE_OUTPUT "--reword incompatible with $1" "
+               echo 'fatal: cannot combine --reword with $1' >expect &&
+               test_must_fail git commit --reword $1 2>actual &&
+               test_cmp expect actual
+       "
+}

These error messages are subject to localization, so you'd want to use test_i18ncmp() here, I think.

Same comment for other new tests.

Quoted 4 lines · Eric Sunshine

Nit: It feels a bit odd (though not outright wrong) to disallow --reword in combination with --amend and --only after the documentation states that --reword is equivalent to using those options.

Yeah I decided to be quite strict, I'm in two minds about the documentation, I think it might be better to remove that line.

I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather than grep so I could check the exact output. [...]

Quoted 5 lines

Quoted 5 lines

t/t7501-commit-basic-functionality.sh +3 -0

@@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '
+test_reword_opt () {
+       test_expect_success C_LOCALE_OUTPUT "--reword incompatible with $1" "
+               echo 'fatal: cannot combine --reword with $1' >expect &&

Quoted 8 lines

t/t7501-commit-basic-functionality.sh test_expect_success '--dry-run --short' ' · line 739

Quoted 4 lines

t/t7501-commit-basic-functionality.sh +4 -0

+               test_must_fail git commit --reword $1 2>actual &&
+               test_cmp expect actual
+       "
+}

These error messages are subject to localization, so you'd want to use
test_i18ncmp() here, I think.
Same comment for other new tests.

I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather than grep so I could check the exact output.

... I should probably check that nothing is printed to stdout in these tests

Quoted 14 lines · Phillip Wood

Quoted 5 lines

Quoted 5 lines

t/t7501-commit-basic-functionality.sh +3 -0

@@ -713,4 +713,60 @@ test_expect_success '--dry-run --short' '
+test_reword_opt () {
+       test_expect_success C_LOCALE_OUTPUT "--reword incompatible with $1" "
+               echo 'fatal: cannot combine --reword with $1' >expect &&

Quoted 8 lines

t/t7501-commit-basic-functionality.sh test_expect_success '--dry-run --short' ' · line 739

Quoted 4 lines

t/t7501-commit-basic-functionality.sh +4 -0

+               test_must_fail git commit --reword $1 2>actual &&
+               test_cmp expect actual
+       "
+}

These error messages are subject to localization, so you'd want to use
test_i18ncmp() here, I think.
Same comment for other new tests.

Quoted 2 lines

I decided to use the C_LOCALE_OUTPUT prerequisite and test_cmp rather than grep so I could check the exact output.

I do not think it is a good idea. Dropping the C_LOCALE_OUTPUT prerequisite and using test_i18ncmpw would be more appropriate.

A test run without GIT_TEST_GETTEXT_POISON will do the byte-for-byte comparison like test_cmp. It is only the poison test, whose purpose is to catch commands that by mistake translated their messages, that would want to mark a test that checks end-user facing messages like this one as special with test_i18ncmp.

Quoted 2 lines

... I should probably check that nothing is printed to stdout in these tests

Perhaps, but that is not the point of "do we diagnose options thare are incompatble with --reword?" test.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

+test_reword_opt () {
...

... I should probably check that nothing is printed to stdout in these tests

Perhaps, but that is not the point of "do we diagnose options thare are incompatble with --reword?" test.

I think it depends if one views the test as checking "do we diagnose options there are incompatible with --reword?" or "what do we show the user when there are options that are incompatible with --reword". For the former we just want to check that the correct error message is printed, for the latter we want to check that only what we expect to be printed is actually printed.

If one notices a typo in the last commit ...

Quoted 1 line · Phillip Wood via GitGitGadget

If one notices a typo in the last commit ...

In the title "commit: add an option the reword HEAD", I would suggest s/the/to/ :-)

From: Phillip Wood <[email protected]>

If one notices a typo in the last commit after starting to stage changes for the next commit it is useful to be able to reword the last commit without changing its contents. Currently the way to do that is by specifying --amend --only with no pathspec which is not that obvious to new users (so much so that before beb635ca9c ("commit:
remove 'Clever' message for --only --amend", 2016-12-09) commit printed a message to congratulate the user on figuring out how to do it). If the last commit is empty one has to pass --allow-empty as well even though the contents are not being changed. This commits adds a --reword option for commit that rewords the last commit without changing its contents.

Hi Phillip,

Quoted 13 lines · Phillip Wood via GitGitGadget

From: Phillip Wood <[email protected]>

If one notices a typo in the last commit after starting to stage changes for the next commit it is useful to be able to reword the last commit without changing its contents. Currently the way to do that is by specifying --amend --only with no pathspec which is not that obvious to new users (so much so that before beb635ca9c ("commit:
remove 'Clever' message for --only --amend", 2016-12-09) commit printed a message to congratulate the user on figuring out how to do it). If the last commit is empty one has to pass --allow-empty as well even though the contents are not being changed. This commits adds a --reword option for commit that rewords the last commit without changing its contents.

I would like to explain the idea I tried to get across when I proposed to implement support for reword! (and --reword) because I feel that it will change the design of this patch in a rather big way.

First of all, let me explain the scenario in which I long for the --reword option: I maintain several patch thickets, the most obvious one being Git for Windows' patch thicket that is merge-rebased [*1*] onto every new Git version.

At times, I need to adjust a commit message in that patch thicket. It would be quite wasteful to perform a full merge-rebase, therefore I typically call git commit --squash <commit> -c <commit>, copy the oneline, paste it after the squash! line (surrounded by empty lines), and then reword the commit message. When the next Git version comes out, I do a merging-rebase, and when the editor pops up because of that squash! oneline, I remove the now-obsolete version(s) of the commit message.

Obviously, I have to be careful to either also pass --only (which I somehow managed to learn about only today) or I have to make sure that I have no staged changes. In practice, I actually specify a bogus path, which has the same effect as --only.

What I would actually rather have is the --reword option: git commit --reword <commit>. In my mind, this would _add_ a new, "empty" commit, letting me edit the commit message of the specified commit, and using that as commit message, prefixed with the line reword! <oneline>.

This, in turn, would need to be accompanied by support in the interactive rebase, to perform the desired reword (which is admittedly quite a bit different from what the way the todo command reword works).

With that in mind, I would like to caution against the design of your current patch, because it would slam the door shut on the way I would like --reword to work.

Ciao,
Dscho

Footnote *1*: In Git for Windows, I want to not only rebase the patches (so that they are as ready to be submitted to the Git mailing list as they can be) but I also want the commit history to fast-forward. The strategy I settled on is the "merging rebase": it is a rebase that starts with a fake merge of the previous commit history, i.e. merging it in using -s ours so that only the commit history comes in, but not the changes. This allows contributors to pull without problems, but also provides the benefits of having a rebased version of the patches. The price is a rather big commit history on top of Git's main branch, as Git for Windows' main branch contains not only the newest iteration of its patches, but _all_ iterations (at least since the first merging-rebase).

Quoted 44 lines · Johannes Schindelin

Quoted 13 lines · Phillip Wood via GitGitGadget

From: Phillip Wood <[email protected]>

If one notices a typo in the last commit after starting to stage changes for the next commit it is useful to be able to reword the last commit without changing its contents. Currently the way to do that is by specifying --amend --only with no pathspec which is not that obvious to new users (so much so that before beb635ca9c ("commit:
remove 'Clever' message for --only --amend", 2016-12-09) commit printed a message to congratulate the user on figuring out how to do it). If the last commit is empty one has to pass --allow-empty as well even though the contents are not being changed. This commits adds a --reword option for commit that rewords the last commit without changing its contents.

I would like to explain the idea I tried to get across when I proposed to implement support for reword! (and --reword) because I feel that it will change the design of this patch in a rather big way.

First of all, let me explain the scenario in which I long for the --reword option: I maintain several patch thickets, the most obvious one being Git for Windows' patch thicket that is merge-rebased [*1*] onto every new Git version.

At times, I need to adjust a commit message in that patch thicket. It would be quite wasteful to perform a full merge-rebase, therefore I typically call git commit --squash <commit> -c <commit>, copy the oneline, paste it after the squash! line (surrounded by empty lines), and then reword the commit message. When the next Git version comes out, I do a merging-rebase, and when the editor pops up because of that squash! oneline, I remove the now-obsolete version(s) of the commit message.

Obviously, I have to be careful to either also pass --only (which I somehow managed to learn about only today) or I have to make sure that I have no staged changes. In practice, I actually specify a bogus path, which has the same effect as --only.

What I would actually rather have is the --reword option: git commit --reword <commit>. In my mind, this would _add_ a new, "empty" commit, letting me edit the commit message of the specified commit, and using that as commit message, prefixed with the line reword! <oneline>.

Quoted 7 lines

This, in turn, would need to be accompanied by support in the interactive rebase, to perform the desired reword (which is admittedly quite a bit different from what the way the todo command reword works).

With that in mind, I would like to caution against the design of your current patch, because it would slam the door shut on the way I would like --reword to work.

I'm keen to have an easy way to reword HEAD and a way to implement your reword! idea.

I posted a comment on your gitgitgadget issue about reword! and drop![1] pointing to some patches[2] that implement the reword! idea as amend!. I think we want to be able to fixup a commit and reword it at the same time which is way I chose the name amend! rather than reword! The implementation currently changes git commit --amend to take an optional commit which isn't ideal. I wonder if calling it revise! would be better then we could have git commit --reword to reword HEAD and git commit --revise <commit> to create a commit that will reword and fixup <commit> when the user runs git rebase -i --autostash. fold! is another possibility.

I don't think this patch series stops us implementing something for rebase but it would mean we couldn't use the name reword! unless we allow git commit --reword to take an optional commit which I'm not that keen on.

What do you think to an alternative name?

Hi Phillip,

Quoted 66 lines · Phillip Wood

Quoted 44 lines · Johannes Schindelin

Quoted 13 lines · Phillip Wood via GitGitGadget

From: Phillip Wood <[email protected]>

If one notices a typo in the last commit after starting to stage changes for the next commit it is useful to be able to reword the last commit without changing its contents. Currently the way to do that is by specifying --amend --only with no pathspec which is not that obvious to new users (so much so that before beb635ca9c ("commit:
remove 'Clever' message for --only --amend", 2016-12-09) commit printed a message to congratulate the user on figuring out how to do it). If the last commit is empty one has to pass --allow-empty as well even though the contents are not being changed. This commits adds a --reword option for commit that rewords the last commit without changing its contents.

I would like to explain the idea I tried to get across when I proposed to implement support for reword! (and --reword) because I feel that it will change the design of this patch in a rather big way.

First of all, let me explain the scenario in which I long for the --reword option: I maintain several patch thickets, the most obvious one being Git for Windows' patch thicket that is merge-rebased [*1*] onto every new Git version.

At times, I need to adjust a commit message in that patch thicket. It would be quite wasteful to perform a full merge-rebase, therefore I typically call git commit --squash <commit> -c <commit>, copy the oneline, paste it after the squash! line (surrounded by empty lines), and then reword the commit message. When the next Git version comes out, I do a merging-rebase, and when the editor pops up because of that squash! oneline, I remove the now-obsolete version(s) of the commit message.

Obviously, I have to be careful to either also pass --only (which I somehow managed to learn about only today) or I have to make sure that I have no staged changes. In practice, I actually specify a bogus path, which has the same effect as --only.

What I would actually rather have is the --reword option: git commit --reword <commit>. In my mind, this would _add_ a new, "empty" commit, letting me edit the commit message of the specified commit, and using that as commit message, prefixed with the line reword! <oneline>.

Quoted 7 lines

This, in turn, would need to be accompanied by support in the interactive rebase, to perform the desired reword (which is admittedly quite a bit different from what the way the todo command reword works).

With that in mind, I would like to caution against the design of your current patch, because it would slam the door shut on the way I would like --reword to work.

I'm keen to have an easy way to reword HEAD and a way to implement your reword! idea.

I posted a comment on your gitgitgadget issue about reword! and drop![1] pointing to some patches[2] that implement the reword! idea as amend!. I think we want to be able to fixup a commit and reword it at the same time which is way I chose the name amend! rather than reword! The implementation currently changes git commit --amend to take an optional commit which isn't ideal. I wonder if calling it revise! would be better then we could have git commit --reword to reword HEAD and git commit --revise <commit> to create a commit that will reword and fixup <commit> when the user runs git rebase -i --autostash. fold! is another possibility.

Quoted 5 lines

I don't think this patch series stops us implementing something for rebase but it would mean we couldn't use the name reword! unless we allow git commit --reword to take an optional commit which I'm not that keen on.

What do you think to an alternative name?

I am really worried that the proliferation of confusingly similar options will increase Git's reputation for being awfully hard to use.

Ciao,
Dscho

Quoted 8 lines

Quoted 5 lines

I don't think this patch series stops us implementing something for rebase but it would mean we couldn't use the name reword! unless we allow git commit --reword to take an optional commit which I'm not that keen on.

What do you think to an alternative name?

I am really worried that the proliferation of confusingly similar options
will increase Git's reputation for being awfully hard to use.

...
The reason I'm not keen on having --amend or --reword take an optional commit is that I think it is confusing as it means sometimes that option creates a new commit and sometimes it modifies the last commit furthermore passing --reword=HEAD would not reword HEAD but creates a reword! commit.

If one notices a typo in the last commit after starting to stage changes for the next commit it is useful to be able to reword the last commit without changing its contents. Currently the way to do that is by specifying --amend --only with no pathspec which is not that obvious to new users (so much so that before beb635ca9c ("commit: remove 'Clever' message for --only --amend", 2016-12-09) commit printed a message to congratulate the user on figuring out how to do it).