Hello Junio,
On 2024-02-15 21:41, Junio C Hamano wrote:
"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).
This is what seems most readable to me:
Rename an existing branch `<oldbranch>` to `<newbranch>`; if left
unspecified, `<oldbranch>` defaults to the current branch.
+ 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.
To me, using "clobber" actually doesn't make it more clear. How
about wording it this way:
Renaming fails if branch `<newbranch>` already exists, but `-M`
or `--move --force` can be used to overwrite the contents of the
existing branch `<newbranch>` while renaming.
To me, "overwriting an existing branch" means something like
"deleting an existing branch first, before doing anything else".
Just like in case of overwriting an existing file, which gets
deleted first, which most users are familiar with.
On the other hand, "overwriting the contents of an existing
branch", at least to me, means that the branch isn't deleted first,
but the new branch is "layered" onto it instead, overwriting some
or all of its contents.
Additionally, "its contents" removes the direct link between the
files and the branches, which should make the description future
proof.
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.
Already brought this back for the v2. It was omitted in the v1 by
an honest mistake.
Thanks, both for writing and carefully reading. Removal of
<newbranch> and <oldbranch> from the OPTIONS enumaration is really
good, too.
I'm glad that you're happy with the improvements so far. :)
