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

Commit Message

Linus Arver via GitGitGadget Jan. 16, 2020, 4:09 p.m. UTC

From: Alexandr Miloslavskiy <[e-mail]>

Together with the previous patch, this brings docs for `git stash` to
the common layout used for most other commands (see for example docs for
`git add`, `git commit`, `git checkout`, `git reset`) where all options
are documented in a separate list.

I have decided to use alphabetical sorting in the list of options. Other
docs often sort in order of appearance or order of importance, but in
this case it wouldn't be easy to read the list where options from
multiple sub-commands are mixed together.

There is some text editing done to make old descriptions better fit into
the list-style format.

Signed-off-by: Alexandr Miloslavskiy <[e-mail]>
---
 Documentation/git-stash.txt | 72 ++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 32 deletions(-)

Comments

Junio C Hamano Jan. 21, 2020, 8:21 p.m. UTC | #1

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

Alexandr Miloslavskiy Feb. 10, 2020, 2:47 p.m. UTC | #2

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.

Patch

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.
 +
 The `--patch` option implies `--keep-index`.  You can use 
 `--no-keep-index` to override this.
 
-When pathspec is given to 'git stash push', the new stash entry records the
-modified states only for the files that match the pathspec.  The index
-entries and working tree files are then rolled back to the state in
-HEAD only for these files, too, leaving files that do not match the
-pathspec intact.
+<pathspec>...::
+	The new stash entry records the modified states only for the files
+	that match the pathspec.  The index entries and working tree files
+	are then rolled back to the state in HEAD only for these files,
+	too, leaving files that do not match the pathspec intact.
 
-When no `<stash>` is given, `stash@{0}` is assumed, otherwise `<stash>` must
-be a reference of the form `stash@{<revision>}`.
+<stash>::
+	A reference of the form `stash@{<revision>}`. When no `<stash>` is
+	given, the latest stash is assumed (that is, `stash@{0}`).
 
 DISCUSSION
 ----------