Hello Kristoffer,
On 2024-02-15 20:28, Kristoffer Haugsbakk wrote:
(I’m adding some Cc from the previous round)
Thanks, I realised that to be missing like two seconds after sending
the patch to the list. :/
On Thu, Feb 15, 2024, at 19:42, Dragan Simic wrote:
Move the descriptions of the <oldbranch> and <newbranch> arguments to
the
descriptions of the branch rename and copy operations, where they
naturally
belong. Also, improve the descriptions of these two branch operations
and,
for completeness, describe the outcomes of forced operations.
Describing the arguments together with their respective operations,
instead
of describing them separately in a rather unfortunate attempt to
squeeze more
meaning out of fewer words, flows much better and makes the
git-branch(1)
man page significantly more usable.
Excellent.
Thanks, I'm glad that you like it. :)
The subsequent improvements shall continue this approach by either
dissolving
as many sentences from the "Description" section into the "Options"
section,
or by having those sentences converted into some kind of more readable
and
better flowing prose, as already discussed and outlined. [1][2]
[1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
[2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
Since this is a one-patch series, it wasn’t clear to me what
“subsequent
improvements” was referring to without following the links. By
following
the links it looks like plans have been made to improve this man page
further. Maybe the commit message could either state this intent more
assertively or commit to it less (like “we might in the future…”)? So
that the links become supplementary information instead of seemingly
filling in some blanks.
(If I read this part correctly.)
It refers to the already planned further rework of the git-branch(1) man
page, which I intend to follow, but also leaves a TODO note to anyone
else
looking at the repository history, in case I end up not following the
plan
for some reason. I hope this makes it more clear.
--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.
Makes sense, maybe even something like this for the v2:
Rename an existing branch <oldbranch> to <newbranch>; if not
specified,
<oldbranch> defaults to the current branch.
🔗 1: https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/
+ 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?
Or maybe I just misunderstood this part.
Basically, the internal logic just overwrites the files, whatever the
files actually are, and issues warning messages about that. Frankly,
I'm not sure that having "--force" available is the safest possible
thing,
but it's already there and needs documenting.
Thus, "the files" refers to just that, the files in the destination
branch that pretty much gets garbled after forced operations.
Perhaps it could be worded like this in the v2:
... but you can use `-M` or `--move --force` to overwrite the
contents
of the existing branch <newbranch> while renaming.
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.
Good point, thanks. I'll address that in the v2.
--M::
+-M [<oldbranch>] <newbranch>::
Shortcut for `--move --force`.
--c::
---copy::
- Copy a branch, together with its config and reflog.
+-c [<oldbranch>] <newbranch>::
+--copy [<oldbranch>] <newbranch>::
+ Copy an existing branch <oldbranch>, which if not specified defaults
+ to the current branch, to <newbranch>. The configuration variables
+ for the <oldbranch> branch and its reflog are also copied
appropriately
+ to be used with <newbranch>. Copying fails if branch <newbranch>
+ already exists, but you can use `-C` or `--copy --force` to
overwrite
+ the files in existing branch <newbranch> while copying.
