Re: [PATCH 6/6] git-reset.txt: make modes description more consistent

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

Michael J Gruber <git@drmicha.warpmail.net> writes:

> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> index 40e2fd8..6b60b59 100644
> --- a/Documentation/git-reset.txt
> +++ b/Documentation/git-reset.txt
> @@ -15,13 +15,13 @@ SYNOPSIS
>  DESCRIPTION
>  -----------
>  In the first and second form, copy entries from <commit> to the index.
> -In the third form, set the current branch head to <commit>, optionally
> +In the third form, set the current branch head (HEAD) to <commit>, optionally
>  modifying index and working tree to match.  The <commit> defaults to HEAD
>  in all forms.

In the documentation (not limited to "git reset"), we say "update HEAD"
and expect the reader to understand that (1) when HEAD is not detached,
that updates what commit the current branch points at, (2) when HEAD is
detached, no branch is affected.

But that is asking too much to newcomers who weren't around before we
introduced detached HEAD around 1.5.0 timeframe.  This documentation
glitch is there primarily because the detached HEAD came much later than
all the "updates the branch" concepts were codified and documented for
many commands that advance/modify history, e.g. reset, commit, merge, etc.

We should rectify this, and I think this series is going in the right
direction.

> @@ -39,9 +39,10 @@ This means that `git reset -p` is the opposite of `git add -p` (see
>  linkgit:git-add[1]).
>  
>  'git reset' [--<mode>] [<commit>]::
> -	This form sets the current branch head to <commit> and then
> -	updates index and working tree according to <mode>, which must
> -	be one of the following:
> +	This form sets the current branch head to <commit> and
> +	possibly updates the index (resetting it to the tree of <commit>) and
> +	the working tree depending on <mode>, which
> +	must be one of the following:

Nice.

> @@ -57,22 +58,30 @@ linkgit:git-add[1]).
>  	been updated. This is the default action.
>  
>  --hard::
> -	Matches the working tree and index to that of the tree being
> -	switched to. Any changes to tracked files in the working tree
> -	since <commit> are lost.
> +	Resets the index and working tree. Any changes to tracked files in the
> +	working tree since <commit> are lost.

Explaining what "reset" does using the undefined word "reset" smells
somewhat wrong.  Not that "matches" is any better, but at least the
original says "to that of the tree", which you seem to have lost, so there
is no indication what they are reset _to_.  I don't think the above is a
good change for this reason.

I also would prefer "are discarded" over "are lost".  It is a conscious
decision to use "git reset --hard" by the user in order to _do_ something,
and "discard" is probably more accurate, in the sense that you use the
command because you want to throw it away, than "lose" which implies you
might have wanted to keep but did something wrong.

>  --merge::
> -	Resets the index to match the tree recorded by the named commit,
> -	and updates the files that are different between the named commit
> -	and the current commit in the working tree.
> +	Resets the index and updates the files in the working tree that are
> +	different between <commit> and HEAD, but keeps those which are
> +	different between the index and working tree (i.e. which have unstaged
> +	changes).
> +	If a file that is different between <commit> and the index has unstaged
> +	changes, reset is aborted.

It may be just me, but "unstaged" sounds as if you once have staged and
then changed your mind and took it back.  "... which have changes that
haven't been added" might be clearer.

Again, it is made unclear what these entities are _reset to_ with the
above change.  Same comments apply for your change to --keep.