Hey Dragan

(I’m adding some Cc from the previous round)

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.

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 (local)
[2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u (local)

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.)

--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.

🔗 1: https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/ (local)

+	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.

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.

--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.