Re: [PATCH] branch: rework the descriptions of rename and copy operations
- From
- Dragan Simic <dsimic@manjaro.org>
- Date
- Feb 15, 2024, 19:47 UTC
- Message-ID
- <6d3cf1220823e793a5244d187627ce97@manjaro.org>
- In-Reply-To
- <f9ec6d31-7158-4381-9701-06fcb33f2e83@app.fastmail.com>
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. :/
Show 18 quoted lines
> 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. :)
Show 22 quoted lines
>> 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.
Show 17 quoted lines
>> --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.Show 15 quoted lines
> 🔗 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.Show 7 quoted lines
> 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.
Show 17 quoted lines
>> --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.