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

From: Marc Branchaud <marcnarc@xiplink.com>
Date: Oct 20, 2023, 21:40 UTC
Message-ID: <841c3b59-9e7c-4492-9d66-8af42c3222ea@xiplink.com>
In-Reply-To: <20231020092707.917514-1-oswald.buddenhagen@gmx.de>

On 2023-10-20 05:27, Oswald Buddenhagen wrote:

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

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

Show 37 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..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"...