Re: [PATCH] branch: rework the descriptions of rename and copy operations

> "Kristoffer Haugsbakk" <code@khaugsbakk.name> 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).

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

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