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

[Junio C Hamano <gitster@pobox.com>]

> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 2dedc21997..f75b80a720 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -43,9 +43,6 @@ created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}`
>  is also possible). Stashes may also be referenced by specifying just the
>  stash index (e.g. the integer `n` is equivalent to `stash@{n}`).
>  
> -OPTIONS
> --------
> -
>  push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<pathspec>...]::
>  
>  	Save your local modifications to a new 'stash entry' and roll them
> @@ -152,40 +149,51 @@ store::
>  	reflog.  This is intended to be useful for scripts.  It is
>  	probably not the command you want to use; see "push" above.
>  
> -If the `--all` option is used instead then the
> -ignored files are stashed and cleaned in addition to the untracked files.
> -
> -If the `--include-untracked` option is used, all untracked files are also
> -stashed and then cleaned up with `git clean`, leaving the working directory
> -in a very clean state.
> -
> -If the `--index` option is used, then tries to reinstate not only the working
> -tree's changes, but also the index's ones. However, this can fail, when you
> -have conflicts (which are stored in the index, where you therefore can no
> -longer apply the changes as they were originally).
> -
> -If the `--keep-index` option is used, all changes already added to the
> -index are left intact.
> -
> -With `--patch`, you can interactively select hunks from the diff
> -between HEAD and the working tree to be stashed.  The stash entry is
> -constructed such that its index state is the same as the index state
> -of your repository, and its worktree contains only the changes you
> -selected interactively.  The selected changes are then rolled back
> -from your worktree. See the ``Interactive Mode'' section of
> -linkgit:git-add[1] to learn how to operate the `--patch` mode.
> +OPTIONS
> +-------
> +-a::
> +--all::
> +	All ignored and untracked files are also stashed and then cleaned
> +	up with `git clean`.
> +
> +-u::
> +--include-untracked::
> +	All untracked files are also stashed and then cleaned up with
> +	`git clean`.
> +
> +--index::
> +	Tries to reinstate not only the working tree's changes, but also
> +	the index's ones. However, this can fail, when you have conflicts
> +	(which are stored in the index, where you therefore can no longer
> +	apply the changes as they were originally).
> +
> +-k::
> +--keep-index::
> +--no-keep-index::
> +	All changes already added to the index are left intact.
> +
> +-p::
> +--patch::
> +	Interactively select hunks from the diff between HEAD and the
> +	working tree to be stashed.  The stash entry is constructed such
> +	that its index state is the same as the index state of your
> +	repository, and its worktree contains only the changes you selected
> +	interactively.  The selected changes are then rolled back from your
> +	worktree. See the ``Interactive Mode'' section of linkgit:git-add[1]
> +	to learn how to operate the `--patch` mode.

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.