| Message ID | 20231020092707.917514-1-oswald.buddenhagen@gmx.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RESEND] git-rebase.txt: rewrite docu for fixup/squash (again) | expand |
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"...
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?
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.
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.
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
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
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(-)