Re: [PATCH] checkout doc: clarify command line args for "checkout paths" mode

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

Jonathan Nieder <jrnieder@gmail.com> writes:

>> +'git checkout' [<tree-ish>] [--] <pathspec>...
>> -'git checkout' [-p|--patch] [<tree-ish>] [--] [<paths>...]
>> +'git checkout' (-p|--patch) [<tree-ish>] [--] [<paths>...]
> ...
> Even for this particular form, the current file is inconsistent about
> whether to call the arguments '<paths>...' or '<pathspec>...'.
> "<paths>..." is even more strange when I think about it for a moment,
> since it is the plural of the plural of path --- i.e., it implies that
> each argument names multiple paths.  Why does this patch use
> <pathspec> for the --no-patch case and <paths> for the --patch case?

No particular reason, other than "the original had <paths>, which
was inherited by the line that was minimally modified, while the
line I typed anew has more preferrable <pathspec>" ;-)

>>  DESCRIPTION
>>  -----------
>> @@ -78,20 +79,13 @@ be used to detach HEAD at the tip of the branch (`git checkout
>>  +
>>  Omitting <branch> detaches HEAD at the tip of the current branch.
>>  
>> +'git checkout' [<tree-ish>] [--] <pathspec>...::
>> +	Restore modified or deleted paths in the working tree by
>> +	replacing with their original contents from the index, or
>> +	the contents from a named <tree-ish> (most often a
>
> This sentence is hard to read:
>
> - "Restore ... in" reads oddly, as though this is an art conservation
>   project that takes place in the working tree.  Is the intent that
>   the command will bring the content of those paths back to the
>   working tree, or something else?

The "restore" was taken from the sentence that was buried in the
paragraph removed by this patch.  I think the original used the
word, fully intending to evoke "an art conservation project"
connotation---after all, the user made a mess in the working tree
and a known-to-be-good copy stored in the index or tree is used to
restore the working tree copy.

> To fix that, it could say
>
> 	Replace matching paths in the working tree with content
> 	from the index or the named tree (most often a commit).  The
> 	<tree-ish> argument can be used to specify a specific tree,
> 	commit, or tag that will be used to update the entries for
> 	matching paths in the index before updating the working tree.

It could (and it was what my earlier draft said), but I found it
less clear than necessary.  It does not matter the data goes from
tree-ish to index and then to the working tree, and a more important
thing is to say both are updated from the tree-ish, for example.

> ... aha, now I see the next paragraph where this text came from.  I
> still think I like my example text based on the first paragraph
> better. :)

And I already said the reason why I started from that and improved
it.

>> @@ -101,6 +95,14 @@ specific side of the merge can be checked out of the index by
>>  using `--ours` or `--theirs`.  With `-m`, changes made to the working tree
>>  file can be discarded to re-create the original conflicted merge result.
>>  
>> +'git checkout' (-p|--patch) [<tree-ish>] [--] [<pathspec>...]::
>> +	This is similar to the "check out paths to the working tree
>> +	from either the index or from a tree-ish" mode described
>> +	above, but lets you use the interactive interface to show
>> +	the "diff" output and choose which hunks to use in the
>> +	result.
>
> s/use/apply/, perhaps.

That might sound technically more accurate, but when we consider
that this application is done in reverse, it is probably not such a
good idea to say "apply" here.  The user chooses which one to use,
and the way it is "used" is by applying the hunk in reverse.
"choose the preimage of which hunks to use in the result" may be
more technically accurate, but it is a mouthful.  I dunno.

The interactive UI explains it by saying

	Discard this hunk from worktree?

which is probably the easiest explanation.

> The options section explains --patch again.  Maybe that should point
> back to here to avoid some duplication.

here is another attempt, this time to avoid "Restore" and <paths>...


 Documentation/git-checkout.txt | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index d6399c0af8..088266e7e9 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -13,7 +13,8 @@ SYNOPSIS
 'git checkout' [-q] [-f] [-m] [--detach] <commit>
 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
-'git checkout' [-p|--patch] [<tree-ish>] [--] [<paths>...]
+'git checkout' [<tree-ish>] [--] <pathspec>...
+'git checkout' (-p|--patch) [<tree-ish>] [--] [<paths>...]
 
 DESCRIPTION
 -----------
@@ -78,20 +79,13 @@ be used to detach HEAD at the tip of the branch (`git checkout
 +
 Omitting <branch> detaches HEAD at the tip of the current branch.
 
-'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::
+'git checkout' [<tree-ish>] [--] <pathspec>...::
 
-	When <paths> or `--patch` are given, 'git checkout' does *not*
-	switch branches.  It updates the named paths in the working tree
-	from the index file or from a named <tree-ish> (most often a
-	commit).  In this case, the `-b` and `--track` options are
-	meaningless and giving either of them results in an error.  The
-	<tree-ish> argument can be used to specify a specific tree-ish
-	(i.e.  commit, tag or tree) to update the index for the given
-	paths before updating the working tree.
-+
-'git checkout' with <paths> or `--patch` is used to restore modified or
-deleted paths to their original contents from the index or replace paths
-with the contents from a named <tree-ish> (most often a commit-ish).
+	Overwrite paths in the working tree by replacing with the
+	contents in the index or in the <tree-ish> (most often a
+	commit).  When a <tree-ish> is given, the paths that
+	match the <pathspec> are updated both in the index and in
+	the working tree.
 +
 The index may contain unmerged entries because of a previous failed merge.
 By default, if you try to check out such an entry from the index, the
@@ -101,6 +95,14 @@ specific side of the merge can be checked out of the index by
 using `--ours` or `--theirs`.  With `-m`, changes made to the working tree
 file can be discarded to re-create the original conflicted merge result.
 
+'git checkout' (-p|--patch) [<tree-ish>] [--] [<pathspec>...]::
+	This is similar to the "check out paths to the working tree
+	from either the index or from a tree-ish" mode described
+	above, but lets you use the interactive interface to show
+	the "diff" output and choose which hunks to use in the
+	result.  See below for the descriptoin of `--patch` option.
+
+
 OPTIONS
 -------
 -q::