From: Dragan Simic <dsimic@manjaro.org>
Date: Tue, 06 Feb 2024 03:16:37 GMT
Subject: Re: [PATCH] branch: clarify <oldbranch> and <newbranch> terms further
Message-ID: <c47dfda08567aa2f315069fb02351ce2@manjaro.org>
In-Reply-To: <CAO_smViHVZRObZjg0tEPXezJZb7wvs9LQdHUFJQTK4-ASCfrmw@mail.gmail.com>

Hello Kyle,

On 2024-02-06 00:53, Kyle Lippincott wrote:
> On Mon, Feb 5, 2024 at 4:45 AM Dragan Simic <dsimic@manjaro.org> wrote:
>> 
>> Clarify further the uses for <oldbranch> and describe the additional 
>> use
>> for <newbranch>.  Mentioning both renaming and copying in these places 
>> might
>> seem a bit redundant, but it should actually make understanding these 
>> terms
>> easier to the readers of the git-branch(1) man page.
>> 
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>  Documentation/git-branch.txt | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/git-branch.txt 
>> b/Documentation/git-branch.txt
>> index 0b0844293235..7392c2f0797d 100644
>> --- a/Documentation/git-branch.txt
>> +++ b/Documentation/git-branch.txt
>> @@ -312,12 +312,14 @@ superproject's "origin/main", but tracks the 
>> submodule's "origin/main".
>>         option is omitted, the current HEAD will be used instead.
>> 
>>  <oldbranch>::
>> -       The name of an existing branch.  If this option is omitted,
>> -       the name of the current branch will be used instead.
>> +       The name of an existing branch to be renamed or copied.
>> +       If this option is omitted, the name of the current branch
>> +       will be used instead.
>> 
>>  <newbranch>::
>> -       The new name for an existing branch. The same restrictions as 
>> for
>> -       <branchname> apply.
>> +       The new name for an existing branch, when renaming a branch,
>> +       or the name for a new branch, when copying a branch.  The same
>> +       naming restrictions apply as for <branchname>.
> 
> The precision here makes me worry that I'm potentially missing
> something when reading this, and has made me re-read it multiple times
> to try to figure out what it is.

Thank you for your feedback!

I'd agree that the first sentence I sent isn't exactly a textbook
example of clarity, :) but it also isn't that bad;  it tries to say
something like "one thing when this, other thing when that".

> I think this would be cleaner:
> 
> The name to give the branch created by the rename or copy operation.
> The operation fails if <newbranch> already exists, use --force to 
> ignore
> this error. The same naming restrictions apply as for <branchname>.
> 
> I'm not super pleased with that second sentence, and maybe we
> shouldn't include it here. Maybe it belongs on the documentation for
> --move and --copy instead? It's sort of mentioned in the text at the
> top describing the -m/-M and -c/-C options, though it's not clear from
> that text what actually happens to the existing copy of <newbranch> if
> one uses --force. If we could include a better description of what
> happens to the existing branch when one uses --force, that'd be nice.

I agree that moving everything to the descriptions of the move and
copy operations, as Junio described further in his message, is a much
better way moving forward.  Describing the results of forced operation
is also needed for completeness.

I'll prepare and send a v2 that takes that approach.

>> 
>>  --sort=<key>::
>>         Sort based on the key given. Prefix `-` to sort in descending
>>