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