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

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

[Oswald Buddenhagen]

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

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

[Phillip Wood]

Hi Oswald

On 23/10/2023 14:00, Oswald Buddenhagen wrote:
> 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"

> +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

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

[Oswald Buddenhagen]

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 
).

>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.

>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

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

[Phillip Wood]

Hi Oswald

On 23/10/2023 18:52, Oswald Buddenhagen wrote:
> 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.

>> 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.

>> 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

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

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> 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.

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

[Taylor Blau]

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

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

[Oswald Buddenhagen]

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

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

[Marc Branchaud]

On 2023-10-23 09:00, Oswald Buddenhagen wrote:
> 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.

> 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.

> +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.

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

[Oswald Buddenhagen]

On Tue, Oct 24, 2023 at 10:01:07AM -0400, Marc Branchaud wrote:
>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.

>> +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.

>> +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".

>> +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

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

[Marc Branchaud]

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.  In 
English "picked" is the right choice.  You should not presume that other 
languages will want to use the word "pick" verbatim.

		M.

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

[Oswald Buddenhagen]

On Fri, Oct 27, 2023 at 08:39:03AM -0400, Marc Branchaud wrote:
>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

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

[Oswald Buddenhagen]

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

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

[Marc Branchaud]

On 2023-10-25 06:29, Oswald Buddenhagen wrote:
> 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.

> 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.

> +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.

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

[Oswald Buddenhagen]

On Fri, Oct 27, 2023 at 09:14:42AM -0400, Marc Branchaud wrote:
>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.

>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

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

[Junio C Hamano]

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.

> 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.

>
>> +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?

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

[Marc Branchaud]

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.

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

[Phillip Wood]

On 27/10/2023 14:14, Marc Branchaud wrote:
>> 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.

> 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