[2/5] doc: reset: unify <pathspec> description
diff mbox series

Message ID 1740ac7a291cfc81418c2d437201c3373487fa26.1572895605.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • Add --pathspec-from-file option for reset, commit
Related show

Commit Message

Elijah Newren via GitGitGadget Nov. 4, 2019, 7:26 p.m. UTC
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

Synchronize it to `git add`, which has a pretty good description.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 Documentation/git-reset.txt | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

Comments

Junio C Hamano Nov. 6, 2019, 4:01 a.m. UTC | #1
"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
writes:

>  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.
Alexandr Miloslavskiy Nov. 6, 2019, 3:56 p.m. UTC | #2
I think I have implemented most suggestions in PatchV2. Thanks!

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

I am a bit confused, is it correct that you don't expect me to change my 
patches?
Junio C Hamano Nov. 7, 2019, 5:46 a.m. UTC | #3
Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> writes:

>> 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).
>
> I am a bit confused, is it correct that you don't expect me to change
> my patches?

I do not know if removal of the only-half-correct "This is the
opposite of add" should be part of this change, or it should be a
separate fix.  The half-wrong sentene was not introduced by this
patch, so leaving it as-is is OK.  It just leaves another thing for
us to think about later.

Thanks.
Alexandr Miloslavskiy Nov. 7, 2019, 11:05 a.m. UTC | #4
On 07.11.2019 6:46, Junio C Hamano wrote:
> I do not know if removal of the only-half-correct "This is the
> opposite of add" should be part of this change, or it should be a
> separate fix.  The half-wrong sentene was not introduced by this
> patch, so leaving it as-is is OK.  It just leaves another thing for
> us to think about later.

My feeling is that this problem is separate from the topic I'm working 
on. I only touched docs to synchronize <pathspec>, so that I can 
copy&paste new option description in next commit without tailoring it to 
local speech. Also, I must admit that upon reading your explanation, I 
felt that I lack the experience to update the writing properly.
Junio C Hamano Nov. 8, 2019, 3:04 a.m. UTC | #5
Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> writes:

> On 07.11.2019 6:46, Junio C Hamano wrote:
>> I do not know if removal of the only-half-correct "This is the
>> opposite of add" should be part of this change, or it should be a
>> separate fix.  The half-wrong sentene was not introduced by this
>> patch, so leaving it as-is is OK.  It just leaves another thing for
>> us to think about later.
>
> My feeling is that this problem is separate from the topic I'm working
> on. I only touched docs to synchronize <pathspec>, so that I can
> copy&paste new option description in next commit without tailoring it
> to local speech. Also, I must admit that upon reading your
> explanation, I felt that I lack the experience to update the writing
> properly.

Yeah, I do think it is cleaner to leave it as a separate issue.

Thanks.

Patch
diff mbox series

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 97e0544d9e..b0ea6e0ce5 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,8 +8,8 @@  git-reset - Reset current HEAD to the specified state
 SYNOPSIS
 --------
 [verse]
-'git reset' [-q] [<tree-ish>] [--] <paths>...
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION
@@ -19,27 +19,33 @@  In the third form, set the current branch head (`HEAD`) to `<commit>`,
 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
+<pathspec>`. This command is equivalent to
+`git restore [--source=<tree-ish>] --staged <pathspec>...`.
++
+After running `git reset <pathspec>` to update the index entry, you can
 use linkgit:git-restore[1] to check the contents out of the index to
 the working tree. Alternatively, using linkgit:git-restore[1]
 and specifying a commit with `--source`, you
 can copy the contents of a path out of a commit to the index and to the
 working tree in one go.
 
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]::
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]::
 	Interactively select hunks in the difference between the index
 	and `<tree-ish>` (defaults to `HEAD`).  The chosen hunks are applied
 	in reverse to the index.
 +
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].
++
 This means that `git reset -p` is the opposite of `git add -p`, i.e.
 you can use it to selectively reset hunks. See the ``Interactive Mode''
 section of linkgit:git-add[1] to learn how to operate the `--patch` mode.