threads / discuss / 60417

[RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

Subject: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

## tl;dr

17 messages between Oct 23, 2023 and Oct 31, 2023.

replies: 16people: 5as markdown or json

Oswald Buddenhagen· Oct 23, 2023, 13:00 UTC · lore

Create a clear top-down structure which makes it hopefully unambiguous what happens when.

Also mention the timestamp along with the author - this is primarily meant to include the keywords somebody might be searching for, like I did a year ago.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
v2:
- slight adjustments inspired by marc. however, i left most things
  unchanged or even went in the opposite direction, because i assume the
  readers to be sufficiently context-sensitive, and the objective is
  merely to be not actively confusing. adding redundancy in the name of
  clarity would just make the text stylistically inferior and arguably
  harder to read.
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Phillip Wood <phillip.wood123@gmail.com>
Cc: Christian Couder <christian.couder@gmail.com>
Cc: Charvi Mendiratta <charvi077@gmail.com>
Cc: Marc Branchaud <marcnarc@xiplink.com>
---
 Documentation/git-rebase.txt | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e7b39ad244..337df9ef2f 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -890,20 +890,21 @@ command "pick" with the command "reword".
 To drop a commit, replace the command "pick" with "drop", or just
 delete the matching line.
 
-If you want to fold two or more commits into one, replace the command
-"pick" for the second and subsequent commits with "squash" or "fixup".
-If the commits had different authors, the folded commit will be
-attributed to the author of the first commit.  The suggested commit
-message for the folded commit is the concatenation of the first
-commit's message with those identified by "squash" commands, omitting the
-messages of commits identified by "fixup" commands, unless "fixup -c"
-is used.  In that case the suggested commit message is only the message
-of the "fixup -c" commit, and an editor is opened allowing you to edit
-the message.  The contents (patch) of the "fixup -c" commit are still
-incorporated into the folded commit. If there is more than one "fixup -c"
-commit, the message from the final one is used.  You can also use
-"fixup -C" to get the same behavior as "fixup -c" except without opening
-an editor.
+If you want to fold two or more commits into one (that is, to combine
+their contents/patches), replace the command "pick" for the second and
+subsequent commits with "squash" or "fixup".
+The commit message for the folded commit is the concatenation of the
+message of the first commit with those of commits identified by "squash"
+commands, omitting those of commits identified by "fixup" commands,
+unless "fixup -c" is used. In the latter case, the message is obtained
+only from the "fixup -c" commit (having more than one of these is
+incorrect).
+If the resulting commit message is a concatenation of multiple messages,
+an editor is opened allowing you to edit it. This is also the case for a
+message obtained via "fixup -c", while using "fixup -C" instead skips
+the editor; this is analogous to the behavior of `git commit`.
+The first commit which contributes to the suggested commit message also
+determines the author, along with the date/timestamp.
 
 `git rebase` will stop when "pick" has been replaced with "edit" or
 when a command fails due to merge errors. When you are done editing

-- 
2.42.0.419.g70bf8a5751
Phillip Wood· Oct 23, 2023, 16:01 UTC · re: Oswald Buddenhagen · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

Hi Oswald
On 23/10/2023 14:00, Oswald Buddenhagen wrote:
Show 31 quoted lines
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..337df9ef2f 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -890,20 +890,21 @@ command "pick" with the command "reword".
>   To drop a commit, replace the command "pick" with "drop", or just
>   delete the matching line.
>   
> -If you want to fold two or more commits into one, replace the command
> -"pick" for the second and subsequent commits with "squash" or "fixup".
> -If the commits had different authors, the folded commit will be
> -attributed to the author of the first commit.  The suggested commit
> -message for the folded commit is the concatenation of the first
> -commit's message with those identified by "squash" commands, omitting the
> -messages of commits identified by "fixup" commands, unless "fixup -c"
> -is used.  In that case the suggested commit message is only the message
> -of the "fixup -c" commit, and an editor is opened allowing you to edit
> -the message.  The contents (patch) of the "fixup -c" commit are still
> -incorporated into the folded commit. If there is more than one "fixup -c"
> -commit, the message from the final one is used.  You can also use
> -"fixup -C" to get the same behavior as "fixup -c" except without opening
> -an editor.
> +If you want to fold two or more commits into one (that is, to combine
> +their contents/patches), replace the command "pick" for the second and
> +subsequent commits with "squash" or "fixup".
> +The commit message for the folded commit is the concatenation of the
> +message of the first commit with those of commits identified by "squash"
> +commands, omitting those of commits identified by "fixup" commands,
> +unless "fixup -c" is used. In the latter case, the message is obtained
> +only from the "fixup -c" commit (having more than one of these is
> +incorrect).

This change is incorrect - it is perfectly fine to have more than one "fixup -c" command. In that case we use the message of the commit of the final "fixup -c" command. One case where there can be multiple "fixup -c" commands is when a commit has been reworded several times via "git commit --fixup=reword:<commit>" and the user runs "git rebase --autosquash"

Show 6 quoted lines
> +If the resulting commit message is a concatenation of multiple messages,
> +an editor is opened allowing you to edit it. This is also the case for a
> +message obtained via "fixup -c", while using "fixup -C" instead skips
> +the editor; this is analogous to the behavior of `git commit`.
> +The first commit which contributes to the suggested commit message also
> +determines the author, along with the date/timestamp.
In the case of

pick A fixup -C B

don't we keep the authorship from A and just use the commit message from B?
Best Wishes
Phillip
Oswald Buddenhagen· Oct 23, 2023, 17:52 UTC · re: Phillip Wood · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

On Mon, Oct 23, 2023 at 05:01:02PM +0100, Phillip Wood wrote:
Show 10 quoted lines
>On 23/10/2023 14:00, Oswald Buddenhagen wrote:
>> +unless "fixup -c" is used. In the latter case, the message is 
>> obtained
>> +only from the "fixup -c" commit (having more than one of these is
>> +incorrect).
>
>This change is incorrect - it is perfectly fine to have more than one 
>"fixup -c" command. In that case we use the message of the commit of the 
>final "fixup -c" command.
>

i know that this is the case, see the previous thread (which i failed to link by header, cf. https://lore.kernel.org/all/20231020092707.917514-1-oswald.buddenhagen@gmx.de/T/#u ).

>One case where there can be multiple "fixup -c" commands is  when a 
>commit has been reworded several times via "git commit 
>--fixup=reword:<commit>" and the user runs "git rebase --autosquash"
>

a cleaner solution would be recognizing the situation and not generating these contradicting commands in the first place. of course that would be more complexity, but it would also allow catching accidental use.

of course i can go back to documenting the status quo, but it seems kind of wrong.

Show 7 quoted lines
>In the case of
>
>pick A
>fixup -C B
>
>don't we keep the authorship from A and just use the commit message from B?
>

uhm. we clearly do. that means i was given incorrect advice in https://lore.kernel.org/all/YjXRM5HiRizZ035p@ugly/T/#u (and so the thread is still looking for a resolution) ...

regards
Phillip Wood· Oct 24, 2023, 09:22 UTC · re: Oswald Buddenhagen · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

Hi Oswald
On 23/10/2023 18:52, Oswald Buddenhagen wrote:
Show 13 quoted lines
> On Mon, Oct 23, 2023 at 05:01:02PM +0100, Phillip Wood wrote:
>> On 23/10/2023 14:00, Oswald Buddenhagen wrote:
>>> +unless "fixup -c" is used. In the latter case, the message is obtained
>>> +only from the "fixup -c" commit (having more than one of these is
>>> +incorrect).
>>
>> This change is incorrect - it is perfectly fine to have more than one 
>> "fixup -c" command. In that case we use the message of the commit of 
>> the final "fixup -c" command.
>>
> i know that this is the case, see the previous thread (which i failed to 
> link by header, cf. 
> https://lore.kernel.org/all/20231020092707.917514-1-oswald.buddenhagen@gmx.de/T/#u ).
Ah, I see Marc has already raised this point.
Show 10 quoted lines
>> One case where there can be multiple "fixup -c" commands is  when a 
>> commit has been reworded several times via "git commit 
>> --fixup=reword:<commit>" and the user runs "git rebase --autosquash"
>>
> a cleaner solution would be recognizing the situation and not generating 
> these contradicting commands in the first place. of course that would be 
> more complexity, but it would also allow catching accidental use.
> 
> of course i can go back to documenting the status quo, but it seems kind 
> of wrong.

I agree there is an argument for improving the implementation of --autosquash but until we do I think it is counterproductive to change the documentation like this as it will cause users to wonder why "rebase --autosquash" generates a todo list that is incorrect according to the documentation.

Show 11 quoted lines
>> In the case of
>>
>> pick A
>> fixup -C B
>>
>> don't we keep the authorship from A and just use the commit message 
>> from B?
>>
> uhm. we clearly do. that means i was given incorrect advice in 
> https://lore.kernel.org/all/YjXRM5HiRizZ035p@ugly/T/#u (and so the 
> thread is still looking for a resolution) ...
I'll take a look at that thread and comment there.

I do think it is a good idea to document where the authorship of a rebased commit comes from.

Best Wishes
Phillip
Junio C Hamano· Oct 24, 2023, 17:19 UTC · re: Phillip Wood · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

Phillip Wood <phillip.wood123@gmail.com> writes:
Show 5 quoted lines
> I agree there is an argument for improving the implementation of
> --autosquash but until we do I think it is counterproductive to change
> the documentation like this as it will cause users to wonder why
> "rebase --autosquash" generates a todo list that is incorrect
> according to the documentation.
That's a good point.
> I do think it is a good idea to document where the authorship of a
> rebased commit comes from.

Yeah, sounds like a good idea. As to the authorship information, it might be nicer if the "rebase -i" insn language supported an option to trigger --reset-author (or even better, --author=...) action for a single commit, but I presume that it is rather a rare event, and as long as people understand that they can stop the sequencing (e.g., an "edit" of the commit would do) and run "commit --amend", it should be OK, so it probably is OK to leave it as-is.

Thanks.
Taylor Blau· Oct 23, 2023, 16:59 UTC · re: Oswald Buddenhagen · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

On Mon, Oct 23, 2023 at 03:00:16PM +0200, Oswald Buddenhagen wrote:
> ---
>  Documentation/git-rebase.txt | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)

The new documentation below looks fine, and I don't have strong feelings beyond the proposed modifications.

The line wrapping is a little odd: it looks like each sentence begins on a its own line. Did you mean for there to be a visual separation between those sentences in the rendered doc? If so, replace the single line feed with a pair of them.

If not, this looks good to me as-is.

Thanks, Taylor

Oswald Buddenhagen· Oct 24, 2023, 21:31 UTC · re: Taylor Blau · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

On Mon, Oct 23, 2023 at 12:59:49PM -0400, Taylor Blau wrote:
>The line wrapping is a little odd: it looks like each sentence begins 
>on a its own line.
>
this is indeed the case.
>Did you mean for there to be a visual separation between those 
>sentences in the rendered doc?
>
no.

the idea is to keep the churn down in later edits, by making reflowing the entire paragraph visibly unnecesary. i can change it if it's deemed too weird.

regards
Marc Branchaud· Oct 24, 2023, 14:01 UTC · re: Oswald Buddenhagen · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

On 2023-10-23 09:00, Oswald Buddenhagen wrote:
Show 17 quoted lines
> Create a clear top-down structure which makes it hopefully unambiguous
> what happens when.
> 
> Also mention the timestamp along with the author - this is primarily
> meant to include the keywords somebody might be searching for, like I
> did a year ago.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> 
> ---
> v2:
> - slight adjustments inspired by marc. however, i left most things
>    unchanged or even went in the opposite direction, because i assume the
>    readers to be sufficiently context-sensitive, and the objective is
>    merely to be not actively confusing. adding redundancy in the name of
>    clarity would just make the text stylistically inferior and arguably
>    harder to read.

I disagree with this on many levels, but your tone seems to brook no discussion and I do not want to get into a protracted debate here.

I will only say that, I personally don't read man pages from start-to-end like a novel. I jump to the part that explains the thing I need to learn about. So I think your assumptions about what context a reader might have in mind when they see this text are invalid.

Since we have very different notions about who is reading this, I think we'll never agree on the final wording. I'll continue to make my suggestions, but I won't stand in the way of these changes if I'm the only one who thinks they could be better.

Show 34 quoted lines
> Cc: Junio C Hamano <gitster@pobox.com>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> Cc: Christian Couder <christian.couder@gmail.com>
> Cc: Charvi Mendiratta <charvi077@gmail.com>
> Cc: Marc Branchaud <marcnarc@xiplink.com>
> ---
>   Documentation/git-rebase.txt | 29 +++++++++++++++--------------
>   1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..337df9ef2f 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -890,20 +890,21 @@ command "pick" with the command "reword".
>   To drop a commit, replace the command "pick" with "drop", or just
>   delete the matching line.
>   
> -If you want to fold two or more commits into one, replace the command
> -"pick" for the second and subsequent commits with "squash" or "fixup".
> -If the commits had different authors, the folded commit will be
> -attributed to the author of the first commit.  The suggested commit
> -message for the folded commit is the concatenation of the first
> -commit's message with those identified by "squash" commands, omitting the
> -messages of commits identified by "fixup" commands, unless "fixup -c"
> -is used.  In that case the suggested commit message is only the message
> -of the "fixup -c" commit, and an editor is opened allowing you to edit
> -the message.  The contents (patch) of the "fixup -c" commit are still
> -incorporated into the folded commit. If there is more than one "fixup -c"
> -commit, the message from the final one is used.  You can also use
> -"fixup -C" to get the same behavior as "fixup -c" except without opening
> -an editor.
> +If you want to fold two or more commits into one (that is, to combine
> +their contents/patches), replace the command "pick" for the second and
> +subsequent commits with "squash" or "fixup".
s/the command "pick"/the "pick" command/
> +The commit message for the folded commit is the concatenation of the
> +message of the first commit with those of commits identified by "squash"
s/message of the first commit/picked commit's message/
> +commands, omitting those of commits identified by "fixup" commands,
> +unless "fixup -c" is used. In the latter case, the message is obtained
> +only from the "fixup -c" commit (having more than one of these is
> +incorrect).

As Phillip said, this is wrong. I agree with Phillip that the documentation should reflect the actual implementation, not what we hope the implementation might be some day.

Show 5 quoted lines
> +If the resulting commit message is a concatenation of multiple messages,
> +an editor is opened allowing you to edit it. This is also the case for a
> +message obtained via "fixup -c", while using "fixup -C" instead skips
> +the editor; this is analogous to the behavior of `git commit`.
> +The first commit which contributes to the suggested commit message also
s/suggested/folded/ -- with "fixup -C" there is no "suggested" message.
Thanks,
		M.
Oswald Buddenhagen· Oct 24, 2023, 21:19 UTC · re: Marc Branchaud · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

On Tue, Oct 24, 2023 at 10:01:07AM -0400, Marc Branchaud wrote:
Show 5 quoted lines
>I will only say that, I personally don't read man pages from 
>start-to-end like a novel.  I jump to the part that explains the thing 
>I need to learn about.  So I think your assumptions about what context 
>a reader might have in mind when they see this text are invalid.
>

we are speaking about the context of a single paragraph, so that doesn't seem like a relevant objection.

Show 5 quoted lines
>> +The commit message for the folded commit is the concatenation of the
>> +message of the first commit with those of commits identified by "squash"
>
>s/message of the first commit/picked commit's message/
>

that does indeed sound better, but i think it's more confusing (and potentially even more so when translated directly). i guess one could use "pick'd commit's", but that's kind of ugly again.

Show 9 quoted lines
>> +commands, omitting those of commits identified by "fixup" commands,
>> +unless "fixup -c" is used. In the latter case, the message is obtained
>> +only from the "fixup -c" commit (having more than one of these is
>> +incorrect).
>
>As Phillip said, this is wrong.  I agree with Phillip that the 
>documentation should reflect the actual implementation, not what we hope 
>the implementation might be some day.
>

there is also the middle ground of making it intentionally vague in anticipation of a possible change. my current draft says "if multiple are present, the last one takes precedence, but this should not be relied upon".

Show 5 quoted lines
>> +The first commit which contributes to the suggested commit message 
>> also
>
>s/suggested/folded/ -- with "fixup -C" there is no "suggested" message.
>

that's a good point, but i want to emphasize the fact that it's the pre-edit message, i.e., that trimming down the squashed message doesn't change anything. anyway, this part will be postponed to another contribution anyway (see parallel thread).

thanks
Marc Branchaud· Oct 27, 2023, 12:39 UTC · re: Oswald Buddenhagen · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

On 2023-10-24 17:19, Oswald Buddenhagen wrote:
Show 10 quoted lines
> 
>>> +The commit message for the folded commit is the concatenation of the
>>> +message of the first commit with those of commits identified by 
>>> "squash"
>>
>> s/message of the first commit/picked commit's message/
>>
> that does indeed sound better, but i think it's more confusing (and 
> potentially even more so when translated directly). i guess one could 
> use "pick'd commit's", but that's kind of ugly again.

Let the translators worry about how to phrase it in other languages. In English "picked" is the right choice. You should not presume that other languages will want to use the word "pick" verbatim.

		M.
Oswald Buddenhagen· Oct 27, 2023, 13:08 UTC · re: Marc Branchaud · lore

Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)

On Fri, Oct 27, 2023 at 08:39:03AM -0400, Marc Branchaud wrote:
Show 14 quoted lines
>On 2023-10-24 17:19, Oswald Buddenhagen wrote:
>>>> +The commit message for the folded commit is the concatenation of 
>>>> the
>>>> +message of the first commit with those of commits identified by 
>>>> "squash"
>>>
>>> s/message of the first commit/picked commit's message/
>>>
>> that does indeed sound better, but i think it's more confusing (and 
>> potentially even more so when translated directly). i guess one could 
>> use "pick'd commit's", but that's kind of ugly again.
>
>Let the translators worry about how to phrase it in other languages.
>

my experience tells me that this isn't a good idea. translations are often done by people who have little domain knowledge of what they translate. it's a good idea to guide them. also, the english text is often read by people who barely understand english, and will attempt literal translations in their head.

>In English "picked" is the right choice.
>

the squashed commits also fit the natural use of "picked", because "picking" means "selecting". it's not advisable to use this potentially ambiguous term when there is an unambiguous alternative way to identify the commit available.

>You should not presume that other languages will want to use the word 
>"pick" verbatim.
>

well, i actually should, because it's the command's own name, which definitely shouldn't be translated.

regards
Oswald Buddenhagen· Oct 25, 2023, 10:29 UTC · re: Oswald Buddenhagen · lore

[PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)

Create a clear top-down structure which makes it hopefully unambiguous what happens when.

The behavior in the presence of multiple "fixup -c" is somewhat questionable, as arguably it would be better to complain about it rather than letting the last instance win. But for the time being we document the status quo, with a note that it is not guaranteed. Note that actually changing it would require --autosquash eliding the superseded uses.

Also emphasize that the author info of the first commit is preserved even in the presence of "fixup -c", as this diverges from "git commit -c"'s behavior. New options matching the latter should be introduced for completeness.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
v3:
- adjust to reality, and elaborate in the commit message why it's
  arguably somewhat suboptimal

i deliberated the 'command "pick"' word order swap suggested by marc, but while it improves things locally, it somehow doesn't flow with the "redundancy-reduced" last part of the sentence.

v2:
- slight adjustments inspired by marc. however, i left most things
  unchanged or even went in the opposite direction, because i assume the
  readers to be sufficiently context-sensitive, and the objective is
  merely to be not actively confusing. adding redundancy in the name of
  clarity would just make the text stylistically inferior and arguably
  harder to read.
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Phillip Wood <phillip.wood123@gmail.com>
Cc: Taylor Blau <me@ttaylorr.com>
Cc: Christian Couder <christian.couder@gmail.com>
Cc: Charvi Mendiratta <charvi077@gmail.com>
Cc: Marc Branchaud <marcnarc@xiplink.com>
---
 Documentation/git-rebase.txt | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e7b39ad244..578d1d34a6 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -890,20 +890,22 @@ command "pick" with the command "reword".
 To drop a commit, replace the command "pick" with "drop", or just
 delete the matching line.
 
-If you want to fold two or more commits into one, replace the command
-"pick" for the second and subsequent commits with "squash" or "fixup".
-If the commits had different authors, the folded commit will be
-attributed to the author of the first commit.  The suggested commit
-message for the folded commit is the concatenation of the first
-commit's message with those identified by "squash" commands, omitting the
-messages of commits identified by "fixup" commands, unless "fixup -c"
-is used.  In that case the suggested commit message is only the message
-of the "fixup -c" commit, and an editor is opened allowing you to edit
-the message.  The contents (patch) of the "fixup -c" commit are still
-incorporated into the folded commit. If there is more than one "fixup -c"
-commit, the message from the final one is used.  You can also use
-"fixup -C" to get the same behavior as "fixup -c" except without opening
-an editor.
+If you want to fold two or more commits into one (that is, to combine
+their contents/patches), replace the command "pick" for the second and
+subsequent commits with "squash" or "fixup".
+The commit message for the folded commit is the concatenation of the
+message of the first commit with those of commits identified by "squash"
+commands, omitting those of commits identified by "fixup" commands,
+unless "fixup -c" is used. In the latter case, the message is obtained
+only from the "fixup -c" commit (if multiple are present, the last one
+takes precedence, but this should not be relied upon).
+If the resulting commit message is a concatenation of multiple messages,
+an editor is opened allowing you to edit it. This is also the case for a
+message obtained via "fixup -c", while using "fixup -C" instead skips
+the editor; this is analogous to the behavior of `git commit`.
+The author information (including date/timestamp) always comes from
+the first commit; this is the case even if "fixup -c/-C" is used,
+contrary to what `git commit` does.
 
 `git rebase` will stop when "pick" has been replaced with "edit" or
 when a command fails due to merge errors. When you are done editing

-- 
2.42.0.419.g70bf8a5751
Marc Branchaud· Oct 27, 2023, 13:14 UTC · re: Oswald Buddenhagen · lore

Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)

On 2023-10-25 06:29, Oswald Buddenhagen wrote:
Show 9 quoted lines
> Create a clear top-down structure which makes it hopefully unambiguous
> what happens when.
> 
> The behavior in the presence of multiple "fixup -c" is somewhat
> questionable, as arguably it would be better to complain about it rather
> than letting the last instance win. But for the time being we document
> the status quo, with a note that it is not guaranteed. Note that
> actually changing it would require --autosquash eliding the superseded
> uses.

I do not think this kind of editorializing belongs in the commit's message, but this likely isn't the first commit message that expresses an opinion.

Show 65 quoted lines
> Also emphasize that the author info of the first commit is preserved
> even in the presence of "fixup -c", as this diverges from "git commit
> -c"'s behavior. New options matching the latter should be introduced for
> completeness.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> 
> ---
> v3:
> - adjust to reality, and elaborate in the commit message why it's
>    arguably somewhat suboptimal
> 
> i deliberated the 'command "pick"' word order swap suggested by marc,
> but while it improves things locally, it somehow doesn't flow with the
> "redundancy-reduced" last part of the sentence.
> 
> v2:
> - slight adjustments inspired by marc. however, i left most things
>    unchanged or even went in the opposite direction, because i assume the
>    readers to be sufficiently context-sensitive, and the objective is
>    merely to be not actively confusing. adding redundancy in the name of
>    clarity would just make the text stylistically inferior and arguably
>    harder to read.
> 
> Cc: Junio C Hamano <gitster@pobox.com>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> Cc: Taylor Blau <me@ttaylorr.com>
> Cc: Christian Couder <christian.couder@gmail.com>
> Cc: Charvi Mendiratta <charvi077@gmail.com>
> Cc: Marc Branchaud <marcnarc@xiplink.com>
> ---
>   Documentation/git-rebase.txt | 30 ++++++++++++++++--------------
>   1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..578d1d34a6 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -890,20 +890,22 @@ command "pick" with the command "reword".
>   To drop a commit, replace the command "pick" with "drop", or just
>   delete the matching line.
>   
> -If you want to fold two or more commits into one, replace the command
> -"pick" for the second and subsequent commits with "squash" or "fixup".
> -If the commits had different authors, the folded commit will be
> -attributed to the author of the first commit.  The suggested commit
> -message for the folded commit is the concatenation of the first
> -commit's message with those identified by "squash" commands, omitting the
> -messages of commits identified by "fixup" commands, unless "fixup -c"
> -is used.  In that case the suggested commit message is only the message
> -of the "fixup -c" commit, and an editor is opened allowing you to edit
> -the message.  The contents (patch) of the "fixup -c" commit are still
> -incorporated into the folded commit. If there is more than one "fixup -c"
> -commit, the message from the final one is used.  You can also use
> -"fixup -C" to get the same behavior as "fixup -c" except without opening
> -an editor.
> +If you want to fold two or more commits into one (that is, to combine
> +their contents/patches), replace the command "pick" for the second and
> +subsequent commits with "squash" or "fixup".
> +The commit message for the folded commit is the concatenation of the
> +message of the first commit with those of commits identified by "squash"
> +commands, omitting those of commits identified by "fixup" commands,
> +unless "fixup -c" is used. In the latter case, the message is obtained
> +only from the "fixup -c" commit (if multiple are present, the last one
> +takes precedence, but this should not be relied upon).
I like the overall phrasing here.

But I think you should remove the "but this should not be relied upon" phrase. This reads as if Git's current behaviour is undefined, which most definitely is not true.

Even changing this to something like "but this might change in the future" is unhelpful. Everything in Git is subject to change over a long-enough time span, so the same could be said about every aspect of Git.

Until the behaviour actually changes, it's perfectly fine for people to use multiple "fixup -c" commands. There's no reason to scare them off of it.

Show 7 quoted lines
> +If the resulting commit message is a concatenation of multiple messages,
> +an editor is opened allowing you to edit it. This is also the case for a
> +message obtained via "fixup -c", while using "fixup -C" instead skips
> +the editor; this is analogous to the behavior of `git commit`.
> +The author information (including date/timestamp) always comes from
> +the first commit; this is the case even if "fixup -c/-C" is used,
> +contrary to what `git commit` does.
This phrasing is much better.
Thanks for putting up with my pedantry!
		M.
Oswald Buddenhagen· Oct 27, 2023, 16:12 UTC · re: Marc Branchaud · lore

Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)

On Fri, Oct 27, 2023 at 09:14:42AM -0400, Marc Branchaud wrote:
Show 12 quoted lines
>On 2023-10-25 06:29, Oswald Buddenhagen wrote:
>> The behavior in the presence of multiple "fixup -c" is somewhat
>> questionable, as arguably it would be better to complain about it rather
>> than letting the last instance win. But for the time being we document
>> the status quo, with a note that it is not guaranteed. Note that
>> actually changing it would require --autosquash eliding the superseded
>> uses.
>
>I do not think this kind of editorializing belongs in the commit's 
>message, but this likely isn't the first commit message that expresses 
>an opinion.
>

commmit messages should elaborate alternatives considered, which includes ones which depend on changes that can be reasonably expected to possibly happen at some point.

Show 12 quoted lines
>But I think you should remove the "but this should not be relied upon" 
>phrase.  This reads as if Git's current behaviour is undefined, which 
>most definitely is not true.
>
>Even changing this to something like "but this might change in the 
>future" is unhelpful.  Everything in Git is subject to change over a 
>long-enough time span, so the same could be said about every aspect of Git.
>
>Until the behaviour actually changes, it's perfectly fine for people to 
>use multiple "fixup -c" commands.  There's no reason to scare them off 
>of it.
>

things can't change overnight; the resistance even the most trivial behavior changes meet is enormous. so explicitly documenting long in advance that something is subject to change is basically the only way to get it changed at all.

specifically for this feature, there is no reason at all to rely on this behavior when hand-editing the todo list, and occurrences most likely indicate a mistake, which is why i would prefer it to be rejected.

regards
Junio C Hamano· Oct 27, 2023, 23:34 UTC · re: Marc Branchaud · lore

Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)

Marc Branchaud <marcnarc@xiplink.com> writes:
> I do not think this kind of editorializing belongs in the commit's
> message, but this likely isn't the first commit message that expresses
> an opinion.
Thanks for saying this.
Show 14 quoted lines
> I like the overall phrasing here.
>
> But I think you should remove the "but this should not be relied upon"
> phrase.  This reads as if Git's current behaviour is undefined, which
> most definitely is not true.
>
> Even changing this to something like "but this might change in the
> future" is unhelpful.  Everything in Git is subject to change over a
> long-enough time span, so the same could be said about every aspect of
> Git.
>
> Until the behaviour actually changes, it's perfectly fine for people
> to use multiple "fixup -c" commands.  There's no reason to scare them
> off of it.

And that would simplify the description to make it easier to follow by readers who are *not* involved in the development process.

Show 12 quoted lines
>
>> +If the resulting commit message is a concatenation of multiple messages,
>> +an editor is opened allowing you to edit it. This is also the case for a
>> +message obtained via "fixup -c", while using "fixup -C" instead skips
>> +the editor; this is analogous to the behavior of `git commit`.
>> +The author information (including date/timestamp) always comes from
>> +the first commit; this is the case even if "fixup -c/-C" is used,
>> +contrary to what `git commit` does.
>
> This phrasing is much better.
>
> Thanks for putting up with my pedantry!

Thanks for a good review. I guess the patch is very near the finish line?

Marc Branchaud· Oct 31, 2023, 18:48 UTC · re: Junio C Hamano · lore

Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)

On 2023-10-27 19:34, Junio C Hamano wrote:
> 
> Thanks for a good review.  I guess the patch is very near the finish
> line?

Yes. In my mind, all that's needed is to remove the part about "should not be relied upon".

		M.
Phillip Wood· Oct 30, 2023, 09:55 UTC · re: Marc Branchaud · lore

Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)

On 27/10/2023 14:14, Marc Branchaud wrote:
Show 38 quoted lines
>> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
>> index e7b39ad244..578d1d34a6 100644
>> --- a/Documentation/git-rebase.txt
>> +++ b/Documentation/git-rebase.txt
>> @@ -890,20 +890,22 @@ command "pick" with the command "reword".
>>   To drop a commit, replace the command "pick" with "drop", or just
>>   delete the matching line.
>> -If you want to fold two or more commits into one, replace the command
>> -"pick" for the second and subsequent commits with "squash" or "fixup".
>> -If the commits had different authors, the folded commit will be
>> -attributed to the author of the first commit.  The suggested commit
>> -message for the folded commit is the concatenation of the first
>> -commit's message with those identified by "squash" commands, omitting 
>> the
>> -messages of commits identified by "fixup" commands, unless "fixup -c"
>> -is used.  In that case the suggested commit message is only the message
>> -of the "fixup -c" commit, and an editor is opened allowing you to edit
>> -the message.  The contents (patch) of the "fixup -c" commit are still
>> -incorporated into the folded commit. If there is more than one "fixup 
>> -c"
>> -commit, the message from the final one is used.  You can also use
>> -"fixup -C" to get the same behavior as "fixup -c" except without opening
>> -an editor.
>> +If you want to fold two or more commits into one (that is, to combine
>> +their contents/patches), replace the command "pick" for the second and
>> +subsequent commits with "squash" or "fixup".
>> +The commit message for the folded commit is the concatenation of the
>> +message of the first commit with those of commits identified by "squash"
>> +commands, omitting those of commits identified by "fixup" commands,
>> +unless "fixup -c" is used. In the latter case, the message is obtained
>> +only from the "fixup -c" commit (if multiple are present, the last one
>> +takes precedence, but this should not be relied upon).
> 
> I like the overall phrasing here.
> 
> But I think you should remove the "but this should not be relied upon" 
> phrase.  This reads as if Git's current behaviour is undefined, which 
> most definitely is not true.

I agree it would be better to remove that phrase, as you say it makes it sounds like the behaviour cannot be relied on.

Show 7 quoted lines
> Even changing this to something like "but this might change in the 
> future" is unhelpful.  Everything in Git is subject to change over a 
> long-enough time span, so the same could be said about every aspect of Git.
> 
> Until the behaviour actually changes, it's perfectly fine for people to 
> use multiple "fixup -c" commands.  There's no reason to scare them off 
> of it.
Indeed
Best Wishes
Phillip

← back to recent threads