Re: [PATCH 2/5] doc: reset: unify <pathspec> description

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 6, 2019, 04:01 UTC
Message-ID: <xmqqh83hveiz.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <1740ac7a291cfc81418c2d437201c3373487fa26.1572895605.git.gitgitgadget@gmail.com>

"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 18 quoted lines

>  optionally modifying index and working tree to match.
>  The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
>  
> -'git reset' [-q] [<tree-ish>] [--] <paths>...::
> -	This form resets the index entries for all `<paths>` to their
> +'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
> +	This form resets the index entries for all `<pathspec>` to their
>  	state at `<tree-ish>`.  (It does not affect the working tree or
>  	the current branch.)
>  +
> -This means that `git reset <paths>` is the opposite of `git add
> -<paths>`. This command is equivalent to
> -`git restore [--source=<tree-ish>] --staged <paths>...`.
> +For more details about the <pathspec> syntax, see the 'pathspec' entry
> +in linkgit:gitglossary[7].
>  +
> -After running `git reset <paths>` to update the index entry, you can
> +This means that `git reset <pathspec>` is the opposite of `git add

It is good to show which document to read more on <pathspec>, but inserting it just before "This means..." breaks the flow of thought.

As we introduce/explain tree-ish and commit used in the synopsis in the ealier part of the description, it probably is a much better place to also introduce/explain pathspec, perhaps like so:

	 In the first and second form, ...
	 In the third form,  ...
	 The <tree-ish>/<commit> defaults to HEAD in all forms.
	+The <pathspec> is used to limit the paths affected by the
	+operation in the first two forms (see the entry for
	+'pathspec' in linkgit:gitglossary[7] for more details).

> +This means that `git reset <pathspec>` is the opposite of `git add
> +<pathspec>`. This command is equivalent to
> +`git restore [--source=<tree-ish>] --staged <pathspec>...`.

Any time I see "... X. This means Y." either in the doc or in the proposed log message, I wish the author (not you in this case, obviously) thought twice about rewriting so that they do not say one thing and immediately have to rephrase it, i.e. either just say Y without saying X, or saying X more clearly without having to say Y.

In this case, however, I think X and Y are related but both relevant. The subcommand resets the index entries for chosen paths to match what is in the tree-ish, which is the same as restoring from a tree to the index.

It is not quite the opposite of adding to the index from the working tree. In this sequence:

	$ edit newfile
	$ git add newfile

and then further

	$ edit newfile
	$ git add newfile
	$ git reset -- newfile

we are taken back to the state _before_ any of the changes made to newfile (in fact, since HEAD does not have newfile, the resulting index would not know about it, either).

Thanks.