threads / patch / 60229

patchdoc: pull: improve rebase=false documentation

Subject: [PATCH] doc: pull: improve rebase=false documentation

## tl;dr

8 messages between Sep 14, 2023 and Sep 16, 2023. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Dragan Simic· Sep 14, 2023, 03:25 UTC · lore
Mention in the documentation that --rebase defaults to false.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 Documentation/git-pull.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Show changes to Documentation/git-pull.txt +2 −1
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 0e14f8b5b2..d28790388e 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -116,7 +116,8 @@ When set to `merges`, rebase using `git rebase --rebase-merges` so that
 the local merge commits are included in the rebase (see
 linkgit:git-rebase[1] for details).
 +
-When false, merge the upstream branch into the current branch.
+When false, merge the upstream branch into the current branch. This is
+the default.
 +
 When `interactive`, enable the interactive mode of rebase.
 +
Junio C Hamano· Sep 14, 2023, 19:00 UTC · re: Dragan Simic · lore

Re: [PATCH] doc: pull: improve rebase=false documentation

Dragan Simic <dsimic@manjaro.org> writes:
> Mention in the documentation that --rebase defaults to false.
I am not quite sure if this is an improvement, though.

It is true that, if you do not have any of your own funny configuration, your "git pull" will *not* rebase.

But that is "if you do not give the --rebase option, you do not trigger the rebase action". Which is quite natural, but it is different from what you wrote above, isn't it?

When people say "the default for `--rebase` is false", they mean this:

    I can say "git pull --rebase=<kind>" to specify how my current
    branch is rebased on top of the upstream.  But if I do not
    specify the <kind>, i.e. "git pull --rebase", the command will
    act as if I gave 'false' as the <kind>.
At least, I would think that is what the word "default" means.

And I would be surprised if the "default" in that sense is 'false'; isn't the default <kind> 'true' --- meaning "perform the plain vanilla 'git rebase'", unless you explicitly asked for 'merges', 'interactive' or 'false'?

After the context of the hunk your patch touches, there is a description on `pull.rebase`. Down there, if you do not set `pull.rebase` or `branch.<name>.rebase` for the current branch at all, the system acts as if you had `false` for these variables. In other words, the default for these variables is `false`, meaning "do not rebase, just merge". But the default option argument for the `--rebase` option given without argument would not be `false`, I would think.

Show 19 quoted lines
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  Documentation/git-pull.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index 0e14f8b5b2..d28790388e 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -116,7 +116,8 @@ When set to `merges`, rebase using `git rebase --rebase-merges` so that
>  the local merge commits are included in the rebase (see
>  linkgit:git-rebase[1] for details).
>  +
> -When false, merge the upstream branch into the current branch.
> +When false, merge the upstream branch into the current branch. This is
> +the default.
>  +
>  When `interactive`, enable the interactive mode of rebase.
>  +
Linus Arver· Sep 14, 2023, 23:57 UTC · re: Junio C Hamano · lore

Re: [PATCH] doc: pull: improve rebase=false documentation

Junio C Hamano <gitster@pobox.com> writes:
Show 36 quoted lines
> Dragan Simic <dsimic@manjaro.org> writes:
>
>> Mention in the documentation that --rebase defaults to false.
>
> I am not quite sure if this is an improvement, though.
>
> It is true that, if you do not have any of your own funny
> configuration, your "git pull" will *not* rebase.  
>
> But that is "if you do not give the --rebase option, you do not
> trigger the rebase action".  Which is quite natural, but it is
> different from what you wrote above, isn't it?
>
> When people say "the default for `--rebase` is false", they mean
> this:
>
>     I can say "git pull --rebase=<kind>" to specify how my current
>     branch is rebased on top of the upstream.  But if I do not
>     specify the <kind>, i.e. "git pull --rebase", the command will
>     act as if I gave 'false' as the <kind>.
>
> At least, I would think that is what the word "default" means.
>
> And I would be surprised if the "default" in that sense is 'false';
> isn't the default <kind> 'true' --- meaning "perform the plain
> vanilla 'git rebase'", unless you explicitly asked for 'merges',
> 'interactive' or 'false'?
>
> After the context of the hunk your patch touches, there is a
> description on `pull.rebase`.  Down there, if you do not set
> `pull.rebase` or `branch.<name>.rebase` for the current branch at
> all, the system acts as if you had `false` for these variables.  In
> other words, the default for these variables is `false`, meaning "do
> not rebase, just merge".  But the default option argument for the
> `--rebase` option given without argument would not be `false`, I
> would think.

I think there are two reasonable interpretations of the word "default" here.

(1) "git pull": This is equivalent to "git pull --rebase=false". So one could say that "git pull" defaults to "--rebase=false".

(2) "git pull --rebase": This is equivalent to "git pull --rebase=true". So one could say that the "--rebase" flag (when it is used as "--rebase" without the "=..." part) defaults to "--rebase=true".

I assume Dragan was thinking of only the meaning in (1). The alternate meaning of (2) which you brought up makes sense too.

I think both flavors of "default" are reasonable interpretations because most (if not all) Git users will start out with (1) in mind as they get familiar with how "git pull" works without any flags (i.e., the "default" git-pull behavior). Eventually they'll learn about (2) and realize how the <kind> defaults to "true" with the "--rebase" flag.

Aside: interestingly, there appears to be a "--no-rebase" option that
means "--rebase=false" (see cd67e4d46b (Teach 'git pull' about --rebase,
2007-11-28)):
       --no-rebase
           This is shorthand for --rebase=false.
Show 19 quoted lines
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>  Documentation/git-pull.txt | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
>> index 0e14f8b5b2..d28790388e 100644
>> --- a/Documentation/git-pull.txt
>> +++ b/Documentation/git-pull.txt
>> @@ -116,7 +116,8 @@ When set to `merges`, rebase using `git rebase --rebase-merges` so that
>>  the local merge commits are included in the rebase (see
>>  linkgit:git-rebase[1] for details).
>>  +
>> -When false, merge the upstream branch into the current branch.
>> +When false, merge the upstream branch into the current branch. This is
>> +the default.
>>  +
>>  When `interactive`, enable the interactive mode of rebase.
>>  +
For reference here is a more complete view of the docs for this flag:
    -r::
    --rebase[=false|true|merges|interactive]::
        When true, rebase the current branch on top of the upstream
        branch after fetching. If there is a remote-tracking branch
        corresponding to the upstream branch and the upstream branch
        was rebased since last fetched, the rebase uses that information
        to avoid rebasing non-local changes.
    +
    When set to `merges`, rebase using `git rebase --rebase-merges` so that
    the local merge commits are included in the rebase (see
    linkgit:git-rebase[1] for details).
    +
    When false, merge the upstream branch into the current branch.
    +
    When `interactive`, enable the interactive mode of rebase.
    +
    See `pull.rebase`, `branch.<name>.rebase` and `branch.autoSetupRebase` in
    linkgit:git-config[1] if you want to make `git pull` always use
    `--rebase` instead of merging.

How about adding something like this instead as the very first paragraph for this flag?

    Supplying "--rebase" defaults to "--rebase=true". Running git-pull
    without arguments implies "--rebase=false", unless relevant
    configuration variables have been set otherwise.

This way, we can discuss what "true" and "false" mean first to get these two flavors of "defaults" sorted out (perhaps also mentioning --no-rebase while we're at it). Then we can discuss the other <kind> values like "merges" and "interactive".

Dragan Simic· Sep 15, 2023, 05:59 UTC · re: Linus Arver · lore

Re: [PATCH] doc: pull: improve rebase=false documentation

On 2023-09-15 01:57, Linus Arver wrote:
Show 106 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>> Dragan Simic <dsimic@manjaro.org> writes:
>> 
>>> Mention in the documentation that --rebase defaults to false.
>> 
>> I am not quite sure if this is an improvement, though.
>> 
>> It is true that, if you do not have any of your own funny
>> configuration, your "git pull" will *not* rebase.
>> 
>> But that is "if you do not give the --rebase option, you do not
>> trigger the rebase action".  Which is quite natural, but it is
>> different from what you wrote above, isn't it?
>> 
>> When people say "the default for `--rebase` is false", they mean
>> this:
>> 
>>     I can say "git pull --rebase=<kind>" to specify how my current
>>     branch is rebased on top of the upstream.  But if I do not
>>     specify the <kind>, i.e. "git pull --rebase", the command will
>>     act as if I gave 'false' as the <kind>.
>> 
>> At least, I would think that is what the word "default" means.
>> 
>> And I would be surprised if the "default" in that sense is 'false';
>> isn't the default <kind> 'true' --- meaning "perform the plain
>> vanilla 'git rebase'", unless you explicitly asked for 'merges',
>> 'interactive' or 'false'?
>> 
>> After the context of the hunk your patch touches, there is a
>> description on `pull.rebase`.  Down there, if you do not set
>> `pull.rebase` or `branch.<name>.rebase` for the current branch at
>> all, the system acts as if you had `false` for these variables.  In
>> other words, the default for these variables is `false`, meaning "do
>> not rebase, just merge".  But the default option argument for the
>> `--rebase` option given without argument would not be `false`, I
>> would think.
> 
> I think there are two reasonable interpretations of the word "default"
> here.
> 
> (1) "git pull": This is equivalent to "git pull --rebase=false". So one
> could say that "git pull" defaults to "--rebase=false".
> 
> (2) "git pull --rebase": This is equivalent to "git pull 
> --rebase=true".
> So one could say that the "--rebase" flag (when it is used as 
> "--rebase"
> without the "=..." part) defaults to "--rebase=true".
> 
> I assume Dragan was thinking of only the meaning in (1). The alternate
> meaning of (2) which you brought up makes sense too.
> 
> I think both flavors of "default" are reasonable interpretations 
> because
> most (if not all) Git users will start out with (1) in mind as they get
> familiar with how "git pull" works without any flags (i.e., the
> "default" git-pull behavior). Eventually they'll learn about (2) and
> realize how the <kind> defaults to "true" with the "--rebase" flag.
> 
> Aside: interestingly, there appears to be a "--no-rebase" option that
> means "--rebase=false" (see cd67e4d46b (Teach 'git pull' about 
> --rebase,
> 2007-11-28)):
> 
>        --no-rebase
>            This is shorthand for --rebase=false.
> 
> For reference here is a more complete view of the docs for this flag:
> 
>     -r::
>     --rebase[=false|true|merges|interactive]::
>         When true, rebase the current branch on top of the upstream
>         branch after fetching. If there is a remote-tracking branch
>         corresponding to the upstream branch and the upstream branch
>         was rebased since last fetched, the rebase uses that 
> information
>         to avoid rebasing non-local changes.
>     +
>     When set to `merges`, rebase using `git rebase --rebase-merges` so 
> that
>     the local merge commits are included in the rebase (see
>     linkgit:git-rebase[1] for details).
>     +
>     When false, merge the upstream branch into the current branch.
>     +
>     When `interactive`, enable the interactive mode of rebase.
>     +
>     See `pull.rebase`, `branch.<name>.rebase` and 
> `branch.autoSetupRebase` in
>     linkgit:git-config[1] if you want to make `git pull` always use
>     `--rebase` instead of merging.
> 
> How about adding something like this instead as the very first 
> paragraph
> for this flag?
> 
>     Supplying "--rebase" defaults to "--rebase=true". Running git-pull
>     without arguments implies "--rebase=false", unless relevant
>     configuration variables have been set otherwise.
> 
> This way, we can discuss what "true" and "false" mean first to get 
> these
> two flavors of "defaults" sorted out (perhaps also mentioning
> --no-rebase while we're at it). Then we can discuss the other <kind>
> values like "merges" and "interactive".

I'm really, really happy to see such a detailed approach to refining the git documentation! Both Junio's and Linus's feedback are highly appreciated.

I totally agree about adding the above-proposed text as the opening paragraph for this option. It's much better than just saying "this is the default", which is admittedly confusing and somewhat misleading.

Junio C Hamano· Sep 15, 2023, 17:43 UTC · re: Linus Arver · lore

Re: [PATCH] doc: pull: improve rebase=false documentation

Linus Arver <linusa@google.com> writes:
Show 13 quoted lines
> Aside: interestingly, there appears to be a "--no-rebase" option that
> means "--rebase=false" (see cd67e4d46b (Teach 'git pull' about --rebase,
> 2007-11-28)):
>
>        --no-rebase
>            This is shorthand for --rebase=false.
> ...
> How about adding something like this instead as the very first paragraph
> for this flag?
>
>     Supplying "--rebase" defaults to "--rebase=true". Running git-pull
>     without arguments implies "--rebase=false", unless relevant
>     configuration variables have been set otherwise.
Phrase nit.
	$ git pull origin

does run the command with arguments. What you mean is "running git-pull without any --rebase arguments implies --no-rebase", but that is saying "not giving --rebase=<any> and not giving --rebase means not rebasing", which makes my head spin.

"--no-rebase" as a command line option does have use to defeat configured pull.rebase that is not set to "false", and allowing "pull.rebase" to be set to "false" does have use to defeat settings for the same variable made by lower-precedence configuration file. "--rebase=false" does not have any reason to exist, except for making the repertoire of "--rebase=<kind>" to be complete.

So, I am still not sure if saying "'git pull' (no other arguments and no configuration) is equivalent to 'git pull --rebase=false'" adds much value.

If --no-rebase and --rebase=false are explained in terms of why these options that specify such an unnatural action (after all, you say "do this" or "do it this way", but do not usually have to say "do not do it that way") need to exist.

If I were writing this patch, I would rearrange the existing text like so:

 * Update the description of "--no-rebase" *NOT* to depend on
   --rebase=false.  Instead move it higher and say
   - The default for "git pull" is to "merge" the other history into
     your history, but optionally you can "rebase" your history on
     top of the other history.
   - There are configuration variables (pull.rebase and
     branch.<name>.rebase) that trigger the optional behaviour, and
     when you set it, your "git pull" would "rebase".
   - The "--no-rebase" option is to defeat such configuration to
     tell the command to "merge" for this particular invocation.
 * Update the description of "--rebase=<kind>" and move the
   paragraph that begins with "When false" to the end, something
   like:
   - `--rebase` alone is equivalent to `--rebase=true`.
   - When set to 'merges'...
   - When set to 'interactive'...
   - See `pull.rebase`, ..., if you want to make `git pull` always
     rebase your history on top of theirs, instead of merging their
     history to yours.
   - `--rebase=false` is synonym to `--no-rebase`.
Junio C Hamano· Sep 15, 2023, 18:00 UTC · re: Junio C Hamano · lore

Re: [PATCH] doc: pull: improve rebase=false documentation

Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> Linus Arver <linusa@google.com> writes:
>
>>     Supplying "--rebase" defaults to "--rebase=true". Running git-pull
>>     without arguments implies "--rebase=false", unless relevant
>>     configuration variables have been set otherwise.
>
> Phrase nit.
>
> 	$ git pull origin
>
> does run the command with arguments.  What you mean is "running
> git-pull without any --rebase arguments implies --no-rebase", but
A nit of my own: "--rebase arguments" -> "--rebase option"
Sorry for the noise ;-)
Linus Arver· Sep 15, 2023, 21:12 UTC · re: Junio C Hamano · lore

Re: [PATCH] doc: pull: improve rebase=false documentation

Junio C Hamano <gitster@pobox.com> writes:
Show 21 quoted lines
> Linus Arver <linusa@google.com> writes:
>
>> Aside: interestingly, there appears to be a "--no-rebase" option that
>> means "--rebase=false" (see cd67e4d46b (Teach 'git pull' about --rebase,
>> 2007-11-28)):
>>
>>        --no-rebase
>>            This is shorthand for --rebase=false.
>> ...
>> How about adding something like this instead as the very first paragraph
>> for this flag?
>>
>>     Supplying "--rebase" defaults to "--rebase=true". Running git-pull
>>     without arguments implies "--rebase=false", unless relevant
>>     configuration variables have been set otherwise.
>
> Phrase nit.
>
> 	$ git pull origin
>
> does run the command with arguments.
Ah, good catch.
> What you mean is "running
> git-pull without any --rebase arguments implies --no-rebase",

Right (modulo your "--rebase arguments" -> "--rebase option" correction in your follow-up email).

> but
> that is saying "not giving --rebase=<any> and not giving --rebase
> means not rebasing", which makes my head spin.
Me too.
> "--no-rebase" as a command line option does have use to defeat
> configured pull.rebase that is not set to "false"

Yes, I noticed this too after digging around a bit more after I sent my message. Thanks for clarifying for the record.

> and allowing
> "pull.rebase" to be set to "false" does have use to defeat settings
> for the same variable made by lower-precedence configuration file.

Indeed! I did not think of this. IOW, Git can be configured in multiple places (the "pull.rebase" setting in ~/.gitconfig can be overridden by the same config in ~/myrepo/.git/config).

> "--rebase=false" does not have any reason to exist, except for
> making the repertoire of "--rebase=<kind>" to be complete.

Agreed. In a sense, the docs for "--rebase=false" should say that it is a synonym for "--no-rebase" (because "--no-rebase" came first, historically), and not the other way around (that "--no-rebase" is shorthand for "--rebase=false").

> So, I am still not sure if saying "'git pull' (no other arguments
> and no configuration) is equivalent to 'git pull --rebase=false'"
> adds much value.

Fair point. That is, there are so many gotchas and "edge case"-like behaviors to consider here, so the statement "'git pull' (no other arguments and no configuration) is equivalent to 'git pull --rebase=false'" is an oversimplification that can be misleading. I agree.

Show 33 quoted lines
> If --no-rebase and --rebase=false are explained in terms of why
> these options that specify such an unnatural action (after all, you
> say "do this" or "do it this way", but do not usually have to say
> "do not do it that way") need to exist.
>
> If I were writing this patch, I would rearrange the existing text
> like so:
>
>  * Update the description of "--no-rebase" *NOT* to depend on
>    --rebase=false.  Instead move it higher and say
>
>    - The default for "git pull" is to "merge" the other history into
>      your history, but optionally you can "rebase" your history on
>      top of the other history.
>
>    - There are configuration variables (pull.rebase and
>      branch.<name>.rebase) that trigger the optional behaviour, and
>      when you set it, your "git pull" would "rebase".
>
>    - The "--no-rebase" option is to defeat such configuration to
>      tell the command to "merge" for this particular invocation.
>
>  * Update the description of "--rebase=<kind>" and move the
>    paragraph that begins with "When false" to the end, something
>    like:
>
>    - `--rebase` alone is equivalent to `--rebase=true`.
>    - When set to 'merges'...
>    - When set to 'interactive'...
>    - See `pull.rebase`, ..., if you want to make `git pull` always
>      rebase your history on top of theirs, instead of merging their
>      history to yours.
>    - `--rebase=false` is synonym to `--no-rebase`.

I think this captures the finer details while still preserving the spirit of Dragan's original patch, so SGTM.

@Dragan if you are OK with doing the (much more substantial) change as suggested, please do. Thanks!

Dragan Simic· Sep 16, 2023, 02:18 UTC · re: Linus Arver · lore

Re: [PATCH] doc: pull: improve rebase=false documentation

On 2023-09-15 23:12, Linus Arver wrote:
Show 111 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>> Linus Arver <linusa@google.com> writes:
>> 
>>> Aside: interestingly, there appears to be a "--no-rebase" option that
>>> means "--rebase=false" (see cd67e4d46b (Teach 'git pull' about 
>>> --rebase,
>>> 2007-11-28)):
>>> 
>>>        --no-rebase
>>>            This is shorthand for --rebase=false.
>>> ...
>>> How about adding something like this instead as the very first 
>>> paragraph
>>> for this flag?
>>> 
>>>     Supplying "--rebase" defaults to "--rebase=true". Running 
>>> git-pull
>>>     without arguments implies "--rebase=false", unless relevant
>>>     configuration variables have been set otherwise.
>> 
>> Phrase nit.
>> 
>> 	$ git pull origin
>> 
>> does run the command with arguments.
> 
> Ah, good catch.
> 
>> What you mean is "running
>> git-pull without any --rebase arguments implies --no-rebase",
> 
> Right (modulo your "--rebase arguments" -> "--rebase option" correction
> in your follow-up email).
> 
>> but
>> that is saying "not giving --rebase=<any> and not giving --rebase
>> means not rebasing", which makes my head spin.
> 
> Me too.
> 
>> "--no-rebase" as a command line option does have use to defeat
>> configured pull.rebase that is not set to "false"
> 
> Yes, I noticed this too after digging around a bit more after I sent my
> message. Thanks for clarifying for the record.
> 
>> and allowing
>> "pull.rebase" to be set to "false" does have use to defeat settings
>> for the same variable made by lower-precedence configuration file.
> 
> Indeed! I did not think of this. IOW, Git can be configured in
> multiple places (the "pull.rebase" setting in ~/.gitconfig can be
> overridden by the same config in ~/myrepo/.git/config).
> 
>> "--rebase=false" does not have any reason to exist, except for
>> making the repertoire of "--rebase=<kind>" to be complete.
> 
> Agreed. In a sense, the docs for "--rebase=false" should say that it is
> a synonym for "--no-rebase" (because "--no-rebase" came first,
> historically), and not the other way around (that "--no-rebase" is
> shorthand for "--rebase=false").
> 
>> So, I am still not sure if saying "'git pull' (no other arguments
>> and no configuration) is equivalent to 'git pull --rebase=false'"
>> adds much value.
> 
> Fair point. That is, there are so many gotchas and "edge case"-like
> behaviors to consider here, so the statement "'git pull' (no other 
> arguments
> and no configuration) is equivalent to 'git pull --rebase=false'" is an
> oversimplification that can be misleading. I agree.
> 
>> If --no-rebase and --rebase=false are explained in terms of why
>> these options that specify such an unnatural action (after all, you
>> say "do this" or "do it this way", but do not usually have to say
>> "do not do it that way") need to exist.
>> 
>> If I were writing this patch, I would rearrange the existing text
>> like so:
>> 
>>  * Update the description of "--no-rebase" *NOT* to depend on
>>    --rebase=false.  Instead move it higher and say
>> 
>>    - The default for "git pull" is to "merge" the other history into
>>      your history, but optionally you can "rebase" your history on
>>      top of the other history.
>> 
>>    - There are configuration variables (pull.rebase and
>>      branch.<name>.rebase) that trigger the optional behaviour, and
>>      when you set it, your "git pull" would "rebase".
>> 
>>    - The "--no-rebase" option is to defeat such configuration to
>>      tell the command to "merge" for this particular invocation.
>> 
>>  * Update the description of "--rebase=<kind>" and move the
>>    paragraph that begins with "When false" to the end, something
>>    like:
>> 
>>    - `--rebase` alone is equivalent to `--rebase=true`.
>>    - When set to 'merges'...
>>    - When set to 'interactive'...
>>    - See `pull.rebase`, ..., if you want to make `git pull` always
>>      rebase your history on top of theirs, instead of merging their
>>      history to yours.
>>    - `--rebase=false` is synonym to `--no-rebase`.
> 
> I think this captures the finer details while still preserving the
> spirit of Dragan's original patch, so SGTM.
> 
> @Dragan if you are OK with doing the (much more substantial) change as
> suggested, please do. Thanks!

Sure, thanks. I'll put together v2 of the patch in the next couple of days, taking into account all the fine details of the awesome feedback provided by both of you, so we can have it discussed and refined further.

← back to recent threads