Re: [PATCH 6/6] git-reset.txt: make modes description more consistent
Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 12 quoted lines
> 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.
Show 11 quoted lines
> @@ -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:
Show 9 quoted lines
> @@ -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.
Show 10 quoted lines
> --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.
Michael J Gruber
