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

[RESEND] 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>

---
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..857e025361 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
+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 the latter case, the message is obtained only
+from the "fixup -c" commit (having more than one "fixup -c" commit
+makes no sense, and only the message from the last one is used).
+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] git-rebase.txt: rewrite docu for fixup/squash (again)

[Marc Branchaud]

On 2023-10-20 05:27, 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>

I think this is a definite improvement to the text.  Thanks for working 
on this!  I do have a few suggestions, below.

> ---
> 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..857e025361 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
> +first commit's message with those identified by "squash" commands,

I think the original text's "those identified by" is a bit vague: Does 
"those" mean "messages" or "commits"?  The sentence reads like "those" 
stands for "messages", but then of course you don't identify *messages* 
with "squash" commands.

Maybe
	those from commits identified by
?

> +omitting the messages 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

I think it's worth emphasizing that "fixup -c" also drops the message 
from the initial "pick" commit as well as all the "squash" commits (I 
presume that's the case, I haven't tried it [1]).

Maybe emphasize the word "only" in the sentence (i.e. spell it as 
'only').  To really drive the point home it could say something like
	obtained 'only' from the "fixup -c" commit, dropping the
	messages of all the other involved commits

> (having more than one "fixup -c" commit
> +makes no sense, and only the message from the last one is used).

"Makes no sense" seems a bit opinionated (although I agree with the 
sentiment).  Also, you can legitimately have more than one "fixup -c" in 
the overall instruction set, as long as there's at least one "pick" 
command in between, e.g.
	pick 1111beef
	fixup 2222f00d
	fixup -c 3333ab1e
	pick 4444d00d
	fixup 5555feed
	fixup -c 6666dead

How about
	(if more than one "fixup -c" command appears in a sequence
	of "fixup" and "squash" commands, only the message from
	the last "fixup -c" commit is used)

Thanks,

		M.

[1] Makes me wonder if rebase should also support "squash -c"...

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

[Junio C Hamano]

Marc Branchaud <marcnarc@xiplink.com> writes:

> [1] Makes me wonder if rebase should also support "squash -c"...

How would it be different from "fixup -c", though?

A "pick" followed by either "fixup -c" or "squash -c" will have the
same effect on the contents (i.e., apply the changes both commits
want to make to their respective parents) and present the end user
an editor buffer filled with the commit specified by the "-c"
option, right?

Or am I missing something?

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

[Marc Branchaud]

On 2023-10-20 19:19, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>> [1] Makes me wonder if rebase should also support "squash -c"...
> 
> How would it be different from "fixup -c", though?
> 
> A "pick" followed by either "fixup -c" or "squash -c" will have the
> same effect on the contents (i.e., apply the changes both commits
> want to make to their respective parents) and present the end user
> an editor buffer filled with the commit specified by the "-c"
> option, right?
> 
> Or am I missing something?

No, you're not.

I should have added "as a convenience".  Squash and fixup are similar 
enough that it seems reasonable for them to both support -c.  Saves 
people from having to remember that only fixup allows -c.

		M.

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

[Junio C Hamano]

Marc Branchaud <marcnarc@xiplink.com> writes:

> I should have added "as a convenience".  Squash and fixup are similar
> enough that it seems reasonable for them to both support -c.  Saves
> people from having to remember that only fixup allows -c.

Yeah, "fixup" could have been a plain "squash" with some option.  It
could have been two options, "-i" ("ignore message of this one") and
"-o" ("use message of this one alone"), and then today's "fixup"
would have been "squash -i", and today's "fixup -c" would have been
"squash -o".

But I agree that "squash -c" is something one may find tempting to
type, after learning "fixup -c".

I forgot to comment on the real contents of your review, by the way.
Everything you said was reasonable.

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

[Oswald Buddenhagen]

On Fri, Oct 20, 2023 at 05:40:01PM -0400, Marc Branchaud wrote:
>I think the original text's "those identified by" is a bit vague: Does 
>"those" mean "messages" or "commits"?  The sentence reads like "those" 
>stands for "messages", but then of course you don't identify *messages* 
>with "squash" commands.
>
fair enough, though the repetition makes it linguistically inferior.

>Maybe emphasize the word "only" in the sentence (i.e. spell it as 
>'only'). 
>
that seems excessive to me. i'm not assuming that my readers are dumb.

> To really drive the point home it could say something like
>	obtained 'only' from the "fixup -c" commit, dropping the
>	messages of all the other involved commits
>
as above.

also, i'm actually uneasy about including the exact behavior in the 
first place, as it codifies something questionable - a better response 
from git would be complaining about it. i will drop it.

>> (having more than one "fixup -c" commit
>> +makes no sense, and only the message from the last one is used).
>
>"Makes no sense" seems a bit opinionated (although I agree with the 
>sentiment).
>
i'm not terribly worried about readers who have an aversion towards 
being told facts ...
anyway, i will use "is incorrect" instead, as it seems more to the 
point.

>Also, you can legitimately have more than one "fixup -c" in the overall 
>instruction set, as long as there's at least one "pick" command in 
>between, e.g.
>
yes, but the context is a single fixup sequence. the above comments 
about readers and repetition apply here, too.

>[1] Makes me wonder if rebase should also support "squash -c"...
>
the distinction is that "squash" combines the messages, while "fixup" 
discards them, and the -c merely changes what is discarded. softening 
that up seems counter-productive to me.

thanks