"Kristoffer Haugsbakk" <code@xxxxxxxxxxxxxxx> writes:
> (If I read this part correctly.)
>
>> --m::
>> ---move::
>> - Move/rename a branch, together with its config and reflog.
>> +-m [<oldbranch>] <newbranch>::
>> +--move [<oldbranch>] <newbranch>::
>> + Rename an existing branch <oldbranch>, which if not specified defaults
>> + to the current branch, to <newbranch>. The configuration variables
>
> I had to read the first sentence a few times in order to understand what
> the “which” part was saying (which seems to come from [1] by Junio). How
> about letting it trail the sentence?
>
> « Rename an existing branch `<oldbranch>` to `<newbranch>`, with
> `<oldbranch>` defaulting to the current branch if not specified.
Yeah, insertion of "if not specified" in the middle made the
resulting text unnecessarily hard to follow, and moving it to the
end is a fine fix. But I think we can just drop it (in other words,
we already said "defaulting to" and that should be sufficient).
>> + for the <oldbranch> branch and its reflog are also renamed appropriately
>> + to be used with <newbranch>. Renaming fails if branch <newbranch>
>> + already exists, but you can use `-M` or `--move --force` to overwrite
>> + the files in existing branch <newbranch> while renaming.
>
> “the files” refers to the branch configuration and the reflog? People
> who read the man pages might not know that the reflogs are implemented
> as files and get tripped up. Maybe “to overwrite” could be left
> unstated?
Yeah, "overwrite the files in existing branch" can be mislead in
other ways, including "in a similar way that 'checkout -f <other>'
can overwrite the files with local modifications in the working tree
with the <other> branch we are switching to". We'd be better off
not mentioning files at all [*].
... but you can use ... to clobber an existing `<newbranch>`.
would be sufficient. If the verb "clobber" is unfamiliar to some
readers, "overwrite" is also fine. The most important part from the
end-user's point of view is that whatever data associated with the
name <newbranch> is now gone, and replaced by what was associated
with the name <oldbranch>. How we stored such data associated with
each branch is immaterial.
Side note: Especially because we are not married to the
files backend that stores one file per branch under
.git/logs/refs/heads/ and .git/refs/heads/ directories
forever. With reftable backend, there is no such files
specific to <newbranch> to be overwritten.
Another thing. The existence of a <newbranch> is not the only case
we fail "git branch -m [<oldbranch>] <newbranch>", but the mention
of this particular safety valve is to tell readers that forcing with
`-M` is how to override the safety. We want to be absolutely clear
that we are not trying to exhaustively enumerate failure modes [*]
to our readers, and I am not sure if we succeeded in the proposed
text.
Side note: In other words, there are other ways the command
can fail, and `-M` may not be a way to "fix" these failures.
> This patch also drops this part. Shouldn’t this be noted?
>
> « , and a reflog entry is created to remember the branch renaming.
>
> Right now it looks like (reads like) the reflog is moved and an entry is
> not made about it.
True. This was missing in my "or something like that" illustration
and the patch copies without checking the original. It should be
resurrected.
Thanks, both for writing and carefully reading. Removal of
<newbranch> and <oldbranch> from the OPTIONS enumaration is really
good, too.
