Re: [PATCH] branch: clarify <oldbranch> and <newbranch> terms further

From: Dragan Simic <dsimic@manjaro.org>
Date: Feb 6, 2024, 03:32 UTC
Message-ID: <8f588db87929b063462dbf4ff134adc7@manjaro.org>
In-Reply-To: <xmqqttmmlahf.fsf@gitster.g>

Hello Junio,

On 2024-02-06 01:09, Junio C Hamano wrote:

Show 26 quoted lines

> Kyle Lippincott <spectral@google.com> writes:
> 
>> 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.
> 
> My preference is to limit the "OPTIONS" section to dashed options.
> If "--move" takes one or two arguments, update its description to
> talk about how these one or two arguments are used, perhaps like
> 
> 	-m [<oldbranch>] <newbranch>::
> 	--move [<oldbranch>] <newbranch>::
> 
> 		Rename an existing branch <oldbranch>, which
>                 defaults to the current branch, to <newbranch>.  The
>                 configuration variables about and the reflog of
>                 <oldbranch> are also renamed appropriately to be
>                 used with <newbranch>. It is an error if <newbranch>
>                 exists (you can use `--force` to clobber an existing
>                 <newbranch>).
> 
> or something like that.

Thank you for your detailed feedback!

I like it and I fully agree that describing the operation arguments fits and flows much better in the descriptions of their respective operations. Describing the outcome of forced operations is also needed for completeness, and for safety.

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

Show 15 quoted lines

> Listing non-options in the list may have been a misguided attempt to
> "save" description on arguments that are common to multiple options,
> but it is not working.  We can see the bad effect of that approach
> only by looking at the current description of the above option,
> which reads:
> 
> 	-m::
> 	--move::
> 		Move/rename a branch, together with its config and reflog.
> 
> It does not mentioning what arguments "--move" takes, and does not
> even refer the readers to the entries for <newbranch> and
> <oldbranch>, so the only plausible way the users can learn what they
> want about this single option is by reading the page from top to
> bottom.

... or the users can perhaps learn by simply experimenting a bit and observing what happens, after getting a bit disappointed by the current descriptions of the operations and resorting to the rather usual "tl;dr" approach.

Avoiding such "tl;dr" scenarios is the way to move forward with the improvements to the git man pages, if you agree.

Show 7 quoted lines

> And trim the DESCRIPTION part. A lot. Because things are explained
> redundantly between there and the OPTIONS part, and their details
> are waiting to drift apart unless we are careful.
> 
> I think I laid all this out and more in a separate message.
> 
> https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/

I agree about this as well, but that will perhaps be handled in some separate patch for the git-branch(1) man page.