Re: [PATCH 4/8] doc: stash: split options from description (2)

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
To: Junio C Hamano <gitster@pobox.com>,
	Alexandr Miloslavskiy via GitGitGadget  <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
Subject: Re: [PATCH 4/8] doc: stash: split options from description (2)
Date: Mon, 10 Feb 2020 15:47:13 +0100	[thread overview]
Message-ID: <eed07f5d-4554-6d26-4d71-f1f975e0ff12@syntevo.com> (raw)
In-Reply-To: <xmqqblqwa7d3.fsf@gitster-ct.c.googlers.com>

On 21.01.2020 21:21, Junio C Hamano wrote:
> I have a mixed feelings about this approach.  While I am sympathetic
> to the "have a single place to describe all" approach this patch
> takes, the approach needs to be executed with care when subcommands
> do not share much of the options at all.  Those readers who jump to
> the "OPTIONS" section and try to ignore anything outside the section
> may not easily notice that --keep-index only applies to subcommands
> that creates a new stash, and meaningless to subcommands that lets
> you inspect existing stashes, or apply one to the working tree (and
> optionally to the index), for example.  If the orinal documentation
> did not use "OPTIONS" as the section header and instead said perhaps
> "SUBCOMMANDS", it would have been even better, but otherwise I would
> suspect that the original "the options understood by 'push' are all
> described under the part that begins with 'push [-p] [-k] ...'
> command line" arrangement was much easier to understand when reading
> them through for the first time to learn and also to find what the
> user is looking for after learning the "concept" (e.g. "with
> 'stash', there is a way to stash-away the changes made to the
> working tree") but before becoming familiar with exact set of
> options for each subcommand (e.g. "and there was an option that let
> me stash only partial changes piecemeal, but what was it spelled?").
> 
> If we were to make the result of "a single place to describe all"
> approach anything useful, I think at least
> 
>   (1) the list itself should make it clear that it does not talk
>       about options related to listing and showing at all,
>       before enumerating dashed options.
> 
>   (2) each item in the enumeration should identify which
>       subcommand(s) accept(s) it.
> 
> So, I dunno.

I have updated the patch with (2). Sorry, I didn't understand what you 
mean in (1).

I have included my reasoning in commit message. If you feel against this 
change, I guess I'll just revert it. Afterall, my only goal was to 
describe new options. Tried to change things because I didn't like how 
this doc goes against the layout I have seen in all previous docs I edited.