git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>,
	Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Subject: Re: [PATCH 4/8] doc: stash: split options from description (2)
Date: Tue, 21 Jan 2020 12:21:12 -0800
Message-ID: <xmqqblqwa7d3.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <708363241f4940e5b627af8519345b762deb77ab.1579190965.git.gitgitgadget@gmail.com> (Alexandr Miloslavskiy via GitGitGadget's message of "Thu, 16 Jan 2020 16:09:21 +0000")

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




  reply	other threads:[~2020-01-21 20:21 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-16 16:09 [PATCH 0/8] Support --pathspec-from-file in rm, stash Alexandr Miloslavskiy via GitGitGadget
2020-01-16 16:09 ` [PATCH 1/8] doc: rm: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2020-01-21 19:14   ` Junio C Hamano
2020-01-16 16:09 ` [PATCH 2/8] rm: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2020-01-21 19:36   ` Junio C Hamano
2020-02-10 14:46     ` Alexandr Miloslavskiy
2020-02-10 18:48       ` Junio C Hamano
2020-02-17 17:25         ` Alexandr Miloslavskiy
2020-01-16 16:09 ` [PATCH 3/8] doc: stash: split options from description (1) Alexandr Miloslavskiy via GitGitGadget
2020-01-16 16:09 ` [PATCH 4/8] doc: stash: split options from description (2) Alexandr Miloslavskiy via GitGitGadget
2020-01-21 20:21   ` Junio C Hamano [this message]
2020-02-10 14:47     ` Alexandr Miloslavskiy
2020-01-16 16:09 ` [PATCH 5/8] doc: stash: document more options Alexandr Miloslavskiy via GitGitGadget
2020-01-21 20:29   ` Junio C Hamano
2020-01-16 16:09 ` [PATCH 6/8] doc: stash: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2020-01-21 20:29   ` Junio C Hamano
2020-01-16 16:09 ` [PATCH 7/8] stash: eliminate crude option parsing Alexandr Miloslavskiy via GitGitGadget
2020-01-16 16:09 ` [PATCH 8/8] stash push: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2020-02-10 14:45 ` [PATCH v2 0/8] Support --pathspec-from-file in rm, stash Alexandr Miloslavskiy via GitGitGadget
2020-02-10 14:45   ` [PATCH v2 1/8] doc: rm: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2020-02-10 14:45   ` [PATCH v2 2/8] rm: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2020-02-10 20:39     ` Junio C Hamano
2020-02-17 17:27       ` Alexandr Miloslavskiy
2020-02-17 17:59         ` Junio C Hamano
2020-02-17 21:03           ` Junio C Hamano
2020-02-10 14:45   ` [PATCH v2 3/8] doc: stash: split options from description (1) Alexandr Miloslavskiy via GitGitGadget
2020-02-10 14:45   ` [PATCH v2 4/8] doc: stash: split options from description (2) Alexandr Miloslavskiy via GitGitGadget
2020-02-10 14:45   ` [PATCH v2 5/8] doc: stash: document more options Alexandr Miloslavskiy via GitGitGadget
2020-02-10 14:45   ` [PATCH v2 6/8] doc: stash: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2020-02-10 14:45   ` [PATCH v2 7/8] stash: eliminate crude option parsing Alexandr Miloslavskiy via GitGitGadget
2020-02-10 14:45   ` [PATCH v2 8/8] stash push: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2020-02-17 17:25   ` [PATCH v3 0/8] Support --pathspec-from-file in rm, stash Alexandr Miloslavskiy via GitGitGadget
2020-02-17 17:25     ` [PATCH v3 1/8] doc: rm: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2020-02-17 17:25     ` [PATCH v3 2/8] rm: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2020-02-17 18:06       ` Alexandr Miloslavskiy
2020-02-17 17:25     ` [PATCH v3 3/8] doc: stash: split options from description (1) Alexandr Miloslavskiy via GitGitGadget
2020-02-17 17:25     ` [PATCH v3 4/8] doc: stash: split options from description (2) Alexandr Miloslavskiy via GitGitGadget
2020-02-17 17:25     ` [PATCH v3 5/8] doc: stash: document more options Alexandr Miloslavskiy via GitGitGadget
2020-02-17 17:25     ` [PATCH v3 6/8] doc: stash: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2020-02-17 17:25     ` [PATCH v3 7/8] stash: eliminate crude option parsing Alexandr Miloslavskiy via GitGitGadget
2020-02-17 17:25     ` [PATCH v3 8/8] stash push: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqblqwa7d3.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=alexandr.miloslavskiy@syntevo.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=ungureanupaulsebastian@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ http://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git