diff mbox series

[v2,9/9] doc/git-rebase: add documentation for fixup [-C|-c] options

Message ID 20210119074102.21598-10-charvi077@gmail.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Charvi Mendiratta Jan. 19, 2021, 7:41 a.m. UTC
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
---
 Documentation/git-rebase.txt | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Marc Branchaud Jan. 19, 2021, 2:37 p.m. UTC | #1
Hi!

On 2021-01-19 2:41 a.m., Charvi Mendiratta wrote:
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Mentored-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com>
> ---
>   Documentation/git-rebase.txt | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index a0487b5cc5..776507e0cc 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -887,9 +887,15 @@ 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 commit
> -messages of the first commit and of those with the "squash" command,
> -but omits the commit messages of commits with the "fixup" command.
> +message for the folded commit is created as follows:
> +
> + - It is made using the commit message of a commit with the "fixup -C"
> +   or "fixup -c" command. In the later case an editor is opened to edit
> +   the commit message.

s/later/latter/

What happens if there is more than one "fixup -C/-c" command?

Thanks for this work!

		M.

> + - Otherwise it's the concatenation of the commit messages of the first
> +   commit and of those with the "squash" command.
> + - It omits the commit messages of commits with the "fixup"
> +   (without -C or -c) command.
>   
>   '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
>
Charvi Mendiratta Jan. 19, 2021, 5:13 p.m. UTC | #2
Hi Marc,

On Tue, 19 Jan 2021 at 20:07, Marc Branchaud <marcnarc@xiplink.com> wrote:
[...]
> >   "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 commit
> > -messages of the first commit and of those with the "squash" command,
> > -but omits the commit messages of commits with the "fixup" command.
> > +message for the folded commit is created as follows:
> > +
> > + - It is made using the commit message of a commit with the "fixup -C"
> > +   or "fixup -c" command. In the later case an editor is opened to edit
> > +   the commit message.
>
> s/later/latter/
>

Thanks, will fix it.

> What happens if there is more than one "fixup -C/-c" command?
>

Upon running interactive rebase, in todo list if we use for example sequence of
commands like `fixup -C`, `fixup -C` , `fixup -C` then it will fixup
content of all and
for commit message it will replace with the commit message of end `fixup -C`

Similarly, if we have sequence like `fixup -c`, `fixup -c`, `fixup -c`
then also it will fixup
up all the content and here it allow user to edit the message, so
opens the editor once
in this case and shows the commit msg of end `fixup -c` to edit and
also contains
commented commit messages of complete fixup chain. So, for any sequence of fixup
chains `fixup -c` works as similar to the `squash` command.

Hope it explains the working.

Thanks and Regards,
Charvi
Marc Branchaud Jan. 19, 2021, 10:05 p.m. UTC | #3
On 2021-01-19 12:13 p.m., Charvi Mendiratta wrote:
> Hi Marc,
> 
> On Tue, 19 Jan 2021 at 20:07, Marc Branchaud <marcnarc@xiplink.com> wrote:
> [...]
>>>    "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 commit
>>> -messages of the first commit and of those with the "squash" command,
>>> -but omits the commit messages of commits with the "fixup" command.
>>> +message for the folded commit is created as follows:
>>> +
>>> + - It is made using the commit message of a commit with the "fixup -C"
>>> +   or "fixup -c" command. In the later case an editor is opened to edit
>>> +   the commit message.
>>
>> s/later/latter/
>>
> 
> Thanks, will fix it.
> 
>> What happens if there is more than one "fixup -C/-c" command?
>>
> 
> Upon running interactive rebase, in todo list if we use for example sequence of
> commands like `fixup -C`, `fixup -C` , `fixup -C` then it will fixup
> content of all and
> for commit message it will replace with the commit message of end `fixup -C`
> 
> Similarly, if we have sequence like `fixup -c`, `fixup -c`, `fixup -c`
> then also it will fixup
> up all the content and here it allow user to edit the message, so
> opens the editor once
> in this case and shows the commit msg of end `fixup -c` to edit and
> also contains
> commented commit messages of complete fixup chain. So, for any sequence of fixup
> chains `fixup -c` works as similar to the `squash` command.
> 
> Hope it explains the working.

Yes, that does explain things.

I'm also under the impression that the "fixup -C/-c" commit's patch is 
still incorporated into the folded commit.  Is that right?

So I think the documentation could be improved.  Assuming that a "fixup 
-C/-c" commit's patch is incorporated just like with plain "fixup", 
something like:

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 last 
last one is used.  You can also use "fixup -C" to get the same behavior 
as "fixup -c" except without opening an editor.

Thanks!

		M.
Charvi Mendiratta Jan. 20, 2021, 7:10 a.m. UTC | #4
On Wed, 20 Jan 2021 at 03:35, Marc Branchaud <marcnarc@xiplink.com> wrote:

[...]
> I'm also under the impression that the "fixup -C/-c" commit's patch is
> still incorporated into the folded commit.  Is that right?
>

yes, it's still incorporated into the folded commit, as it just adds
options to the `fixup` command to replace or edit the commit message.

> So I think the documentation could be improved.  Assuming that a "fixup
> -C/-c" commit's patch is incorporated just like with plain "fixup",
> something like:
>
> 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 last
> last one is used.  You can also use "fixup -C" to get the same behavior
> as "fixup -c" except without opening an editor.
>

I agree, this is now more clear and explains for more than one `fixup
[-C|-c]` or `squash` command also. Thanks for the suggestions, I will
change this in the next revision.

Thanks and Regards,
Charvi
Phillip Wood Jan. 20, 2021, 11:04 a.m. UTC | #5
Hi Charvi

On 19/01/2021 17:13, Charvi Mendiratta wrote:
> Hi Marc,
> 
> On Tue, 19 Jan 2021 at 20:07, Marc Branchaud <marcnarc@xiplink.com> wrote:
> [...]
>>>    "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 commit
>>> -messages of the first commit and of those with the "squash" command,
>>> -but omits the commit messages of commits with the "fixup" command.
>>> +message for the folded commit is created as follows:
>>> +
>>> + - It is made using the commit message of a commit with the "fixup -C"
>>> +   or "fixup -c" command. In the later case an editor is opened to edit
>>> +   the commit message.
>>
>> s/later/latter/
>>
> 
> Thanks, will fix it.
> 
>> What happens if there is more than one "fixup -C/-c" command?
>>
> 
> Upon running interactive rebase, in todo list if we use for example sequence of
> commands like `fixup -C`, `fixup -C` , `fixup -C` then it will fixup
> content of all and
> for commit message it will replace with the commit message of end `fixup -C`
> 
> Similarly, if we have sequence like `fixup -c`, `fixup -c`, `fixup -c`
> then also it will fixup
> up all the content and here it allow user to edit the message, so
> opens the editor once

It is good that we only open the editor once in this case - I'd not 
thought about chains of `fixup -c` before reading this. Do we have a 
test to verify that the editor is only opened once?

Best Wishes

Phillip

> in this case and shows the commit msg of end `fixup -c` to edit and
> also contains
> commented commit messages of complete fixup chain. So, for any sequence of fixup
> chains `fixup -c` works as similar to the `squash` command.
> 
> Hope it explains the working.
> 
> Thanks and Regards,
> Charvi
>
Charvi Mendiratta Jan. 20, 2021, 12:31 p.m. UTC | #6
Hi Phillip,

On Wed, 20 Jan 2021 at 16:34, Phillip Wood <phillip.wood123@gmail.com> wrote:

> >[...]
> > Similarly, if we have sequence like `fixup -c`, `fixup -c`, `fixup -c`
> > then also it will fixup
> > up all the content and here it allow user to edit the message, so
> > opens the editor once
>
> It is good that we only open the editor once in this case - I'd not
> thought about chains of `fixup -c` before reading this. Do we have a
> test to verify that the editor is only opened once?
>

No, we don't. But I also agree, it's a good idea to add a test for it.
I think may be one sequence with 'fixup -C', 'fixup -c', 'fixup -c'
and the other 'squash' , 'fixup -C', 'fixup -c', is sufficient for
testing. Or any other suggestions for testing it ?

Thanks and regards,
Charvi
Phillip Wood Jan. 20, 2021, 2:29 p.m. UTC | #7
Hi Charvi

On 20/01/2021 12:31, Charvi Mendiratta wrote:
> Hi Phillip,
> 
> On Wed, 20 Jan 2021 at 16:34, Phillip Wood <phillip.wood123@gmail.com> wrote:
> 
>>> [...]
>>> Similarly, if we have sequence like `fixup -c`, `fixup -c`, `fixup -c`
>>> then also it will fixup
>>> up all the content and here it allow user to edit the message, so
>>> opens the editor once
>>
>> It is good that we only open the editor once in this case - I'd not
>> thought about chains of `fixup -c` before reading this. Do we have a
>> test to verify that the editor is only opened once?
>>
> 
> No, we don't. But I also agree, it's a good idea to add a test for it.
> I think may be one sequence with 'fixup -C', 'fixup -c', 'fixup -c'
> and the other 'squash' , 'fixup -C', 'fixup -c', is sufficient for
> testing.

Those are both good sequences to test. I think we should check 'fixup 
-c' 'fixup' as well - with 'squash' 'fixup' we open the editor after the 
fixup so the user can see all the changes that will be committed when 
they edit the message, we should do the same for 'fixup -c' 'fixup'. 
Also 'fixup -c' 'squash' might be worth testing as well.

Best Wishes

Phillip


  Or any other suggestions for testing it ?
> 
> Thanks and regards,
> Charvi
>
Charvi Mendiratta Jan. 20, 2021, 4:09 p.m. UTC | #8
On Wed, 20 Jan 2021 at 19:59, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Charvi
>
> On 20/01/2021 12:31, Charvi Mendiratta wrote:
> > Hi Phillip,
> >
> > On Wed, 20 Jan 2021 at 16:34, Phillip Wood <phillip.wood123@gmail.com> wrote:
> >
> >>> [...]
> >>> Similarly, if we have sequence like `fixup -c`, `fixup -c`, `fixup -c`
> >>> then also it will fixup
> >>> up all the content and here it allow user to edit the message, so
> >>> opens the editor once
> >>
> >> It is good that we only open the editor once in this case - I'd not
> >> thought about chains of `fixup -c` before reading this. Do we have a
> >> test to verify that the editor is only opened once?
> >>
> >
> > No, we don't. But I also agree, it's a good idea to add a test for it.
> > I think may be one sequence with 'fixup -C', 'fixup -c', 'fixup -c'
> > and the other 'squash' , 'fixup -C', 'fixup -c', is sufficient for
> > testing.
>
> Those are both good sequences to test. I think we should check 'fixup
> -c' 'fixup' as well - with 'squash' 'fixup' we open the editor after the
> fixup so the user can see all the changes that will be committed when
> they edit the message, we should do the same for 'fixup -c' 'fixup'.
> Also 'fixup -c' 'squash' might be worth testing as well.
>

Agree, I will add these two test cases also and send the next revision of
patches.

Thanks for reviews!

Thanks and Regards,
Charvi
diff mbox series

Patch

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index a0487b5cc5..776507e0cc 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -887,9 +887,15 @@  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 commit
-messages of the first commit and of those with the "squash" command,
-but omits the commit messages of commits with the "fixup" command.
+message for the folded commit is created as follows:
+
+ - It is made using the commit message of a commit with the "fixup -C"
+   or "fixup -c" command. In the later case an editor is opened to edit
+   the commit message.
+ - Otherwise it's the concatenation of the commit messages of the first
+   commit and of those with the "squash" command.
+ - It omits the commit messages of commits with the "fixup"
+   (without -C or -c) command.
 
 '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