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 > [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.) > --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/ > + 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.
