Re: [PATCH 4/3] branch: fix "copy" to never touch HEAD

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Sep 27 2017, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> I do think however that we also need to update the docs, the relevant
>> origin/master...gitster/sd/branch-copy diff is currently:
>>
>>     +The `-c` and `-C` options have the exact same semantics as `-m` and
>>     +`-M`, except instead of the branch being renamed it along with its
>>     +config and reflog will be copied to a new name.
>>
>> Suggestions welcome, but I think that should probably be changed to
>> something like the following as part of this patch:
>>
>>     The `-c` and `-C` options have the exact same semantics as `-m` and
>>     `-M`, except instead of the branch being renamed it along with its
>>     config and reflog will be copied to a new name. Furthermore, unlike
>>     its `-m` and `-M` cousins the `-c` and `-C` options will never move
>>     the HEAD, whereas the move option will do so if the source branch is
>>     the currently checked-out branch.
>
> I do not think anybody even _imagines_ copy to move HEAD, and do not
> think "unlike -m, it doesn't touch HEAD" a worthwhile thing to say.
>
> It is '-m' whose behaviour may look strange wrt HEAD, so _that_ may
> be worth mentioning, though.
>
> I suspect that your reaction probably comes from being too married
> to the idea that HEAD has a string that is the same as the refname
> of the current branch.  But that is a mere implementation detail.
> Users would think that HEAD points at the current branch and does
> not even care how that pointing is implemented.

To cut to the chase instead of pointlessly replying to this
point-by-point, I think your patch quoted below is good and solves the
minor doc issue I had with your patch.

Yes HEAD is an implementation detail, but it's an exposed implementation
detail.

Thus before your patch it was true to say that "[-c] has the exact same
semantics as [-m] [...] except [ s/move/rename/ ]" since that was the
only behavior change, but with your patch adding another "if (!copy &&
...)" we'd now have two things different in the code, but only one thing
enumerated as being different in the docs.

Just rephrasing it as you did is a better way out of that than my
proposed patch. Thanks.

> Conceptually, you can consider that each branch has its own
> identity, separate from various traits it has, like what its
> upstream branch is, what commit it points at, what its reflog says,
> and (most notably) what its name is.
>
> Then you can think of "branch -m old new" to be (1) finding the
> instance of branch that currently has name 'old' and (2) updating
> only one of its trait, namely, its name, without changing anything
> else.  Creating a new instance of a branch by copying an existing
> one, on the other hand, would then be the matter of (1) finding the
> instance of branch with name 'old' and (2) creating another instance
> of branch with the same traits as the original, modulo its name is
> set to 'new'.
>
> A necessary wrinkle for "branch -m" that falls out of the above
> world model is that HEAD needs to be adjusted in order to keep
> pointing at the same (renamed) instance of branch that now has an
> updated name, if HEAD happened to be pointing at the instance of the
> branch whose name trait has been updated.
>
> So, from that point of view, I'd prefer to do something like the
> attached patch instead.  I notice that "branch -m" does not mention
> configuration variables carried over to the new branch, but I do not
> necessarily think we want to exhaustively enumerate what traits are
> carried over here, so perhaps it is OK as is.
>
>  Documentation/git-branch.txt | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index fe029ac6fc..d425e3acd4 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -63,11 +63,13 @@ With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
>  If <oldbranch> had a corresponding reflog, it is renamed to match
>  <newbranch>, and a reflog entry is created to remember the branch
>  renaming. If <newbranch> exists, -M must be used to force the rename
> -to happen.
> +to happen.  If you rename a branch that is currently checked out,
> +`HEAD` is adjusted so that the branch (whose name is now
> +<newbranch>) stays to be the current branch.
>
> -The `-c` and `-C` options have the exact same semantics as `-m` and
> -`-M`, except instead of the branch being renamed it along with its
> -config and reflog will be copied to a new name.
> +With a `-c` or`-C` option, a new branch <newbranch> is created by
> +copying the traits like the reflog contents and `branch.*.*`
> +configuration from an existing <oldbranch>.
>
>  With a `-d` or `-D` option, `<branchname>` will be deleted.  You may
>  specify more than one branch for deletion.  If the branch currently