[PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Torsten Bögershausen]

git checkout <pathspec> can be used to reset changes in the working tree.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Version 2: Try to summarize the suggestions from the mailing list
 Documentation/git-checkout.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index d263a56..39ad36f 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -3,7 +3,7 @@ git-checkout(1)
  NAME
 ----
-git-checkout - Checkout a branch or paths to the working tree
+git-checkout - Switch branches or restore changes
  SYNOPSIS
 --------
@@ -89,6 +89,10 @@ Omitting <branch> detaches HEAD at the tip of the current branch.
 	(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).
++
 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
 checkout operation will fail and nothing will be checked out.
-- 
2.2.0.rc1.790.ge19fcd2

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Duy Nguyen]

On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> -git-checkout - Checkout a branch or paths to the working tree
> +git-checkout - Switch branches or restore changes

I didn't follow closely the previous discussion. Forgive me if this is
already discussed, but I would keep the "in the working tree".
"Restore changes" alone seems vague.
-- 
Duy

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Matthieu Moy]

Duy Nguyen <pclouds@gmail.com> writes:

> On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> -git-checkout - Checkout a branch or paths to the working tree
>> +git-checkout - Switch branches or restore changes
>
> I didn't follow closely the previous discussion.

(Neither did I)

> Forgive me if this is already discussed, but I would keep the "in the
> working tree". "Restore changes" alone seems vague.

"Restore previous version" would be better than "Restore changes" to me.

"changes" sounds like "the diff between a commit and its parent", so it
makes sense to "revert a change" (git revert), but not "restore a
change".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Andres G. Aragoneses]

On 17/06/15 12:54, Matthieu Moy wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>>> -git-checkout - Checkout a branch or paths to the working tree
>>> +git-checkout - Switch branches or restore changes
>>
>> I didn't follow closely the previous discussion.
>
> (Neither did I)
>
>> Forgive me if this is already discussed, but I would keep the "in the
>> working tree". "Restore changes" alone seems vague.
>
> "Restore previous version" would be better than "Restore changes" to me.

"previous version" sounds ambiguous. How about "discard local changes"?

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Matthieu Moy]

"Andres G. Aragoneses" <knocte@gmail.com> writes:

> On 17/06/15 12:54, Matthieu Moy wrote:
>> Duy Nguyen <pclouds@gmail.com> writes:
>>
>>> On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>>>> -git-checkout - Checkout a branch or paths to the working tree
>>>> +git-checkout - Switch branches or restore changes
>>>
>>> I didn't follow closely the previous discussion.
>>
>> (Neither did I)
>>
>>> Forgive me if this is already discussed, but I would keep the "in the
>>> working tree". "Restore changes" alone seems vague.
>>
>> "Restore previous version" would be better than "Restore changes" to me.
>
> "previous version" sounds ambiguous.

Yes, but "git checkout" can do many things. It can restore an old
commited state, restore from the index, ... so we need to either be
vague, or use a long enumeration.

> How about "discard local changes"?

To me this describes "git checkout HEAD", but neither "git checkout --
file" nor "git checkout HEAD^^^".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Andres G. Aragoneses]

On 17/06/15 13:54, Matthieu Moy wrote:
> "Andres G. Aragoneses" <knocte@gmail.com> writes:
>
>> On 17/06/15 12:54, Matthieu Moy wrote:
>>> Duy Nguyen <pclouds@gmail.com> writes:
>>>
>>>> On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>>>>> -git-checkout - Checkout a branch or paths to the working tree
>>>>> +git-checkout - Switch branches or restore changes
>>>>
>>>> I didn't follow closely the previous discussion.
>>>
>>> (Neither did I)
>>>
>>>> Forgive me if this is already discussed, but I would keep the "in the
>>>> working tree". "Restore changes" alone seems vague.
>>>
>>> "Restore previous version" would be better than "Restore changes" to me.
>>
>> "previous version" sounds ambiguous.
>
> Yes, but "git checkout" can do many things. It can restore an old
> commited state, restore from the index, ... so we need to either be
> vague, or use a long enumeration.
>
>> How about "discard local changes"?
>
> To me this describes "git checkout HEAD", but neither "git checkout --
> file" nor "git checkout HEAD^^^".
>

I didn't mean to use just "discard local changes". I was proposing that 
as a replacement to the "restore changes" substring.

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Matthieu Moy]

"Andres G. Aragoneses" <knocte@gmail.com> writes:

> On 17/06/15 13:54, Matthieu Moy wrote:
>> "Andres G. Aragoneses" <knocte@gmail.com> writes:
>>
>>> On 17/06/15 12:54, Matthieu Moy wrote:
>>>> Duy Nguyen <pclouds@gmail.com> writes:
>>>>
>>>>> On Wed, Jun 17, 2015 at 2:54 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>>>>>> -git-checkout - Checkout a branch or paths to the working tree
>>>>>> +git-checkout - Switch branches or restore changes
>>>>>
>>>>> I didn't follow closely the previous discussion.
>>>>
>>>> (Neither did I)
>>>>
>>>>> Forgive me if this is already discussed, but I would keep the "in the
>>>>> working tree". "Restore changes" alone seems vague.
>>>>
>>>> "Restore previous version" would be better than "Restore changes" to me.
>>>
>>> "previous version" sounds ambiguous.
>>
>> Yes, but "git checkout" can do many things. It can restore an old
>> commited state, restore from the index, ... so we need to either be
>> vague, or use a long enumeration.
>>
>>> How about "discard local changes"?
>>
>> To me this describes "git checkout HEAD", but neither "git checkout --
>> file" nor "git checkout HEAD^^^".
>
> I didn't mean to use just "discard local changes". I was proposing
> that as a replacement to the "restore changes" substring.

Yes, but "Switch branchs or discard local changes" still does not
describe "git checkout HEAD^^^ -- file.txt" (restore to an old state,
but does not switch branch) or "git checkout -- file.txt" (get from the
index).

To me, "discard local changes" imply that there will be no uncommited
changes on the files implied in the command after the operation.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Yes, but "Switch branchs or discard local changes" still does not
> describe "git checkout HEAD^^^ -- file.txt" (restore to an old state,
> but does not switch branch) or "git checkout -- file.txt" (get from the
> index).

You are right, especially when file.txt does not have any change
relative to HEAD, there is no "discarding" going on.  You are
actively introducing a change to an unchanged file by checking
contents out of a different revision.

> To me, "discard local changes" imply that there will be no uncommited
> changes on the files implied in the command after the operation.

Yup.

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Torsten Bögershausen]

On 2015-06-17 17.29, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> 
>> Yes, but "Switch branchs or discard local changes" still does not
>> describe "git checkout HEAD^^^ -- file.txt" (restore to an old state,
>> but does not switch branch) or "git checkout -- file.txt" (get from the
>> index).
> 
> You are right, especially when file.txt does not have any change
> relative to HEAD, there is no "discarding" going on.  You are
> actively introducing a change to an unchanged file by checking
> contents out of a different revision.
> 
>> To me, "discard local changes" imply that there will be no uncommited
>> changes on the files implied in the command after the operation.
> 
> Yup.
Thanks for the comments.
I agree that we should keep the headline as is.
What is about the the rest of the patch?
Does it makes sense ?

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Yes, but "Switch branchs or discard local changes" still does not
>> describe "git checkout HEAD^^^ -- file.txt" (restore to an old state,
>> but does not switch branch) or "git checkout -- file.txt" (get from the
>> index).
>
> You are right, especially when file.txt does not have any change
> relative to HEAD, there is no "discarding" going on.  You are
> actively introducing a change to an unchanged file by checking
> contents out of a different revision.
>
>> To me, "discard local changes" imply that there will be no uncommited
>> changes on the files implied in the command after the operation.
>
> Yup.

What was discussed in this thread sounded suspiciously familiar ;-).

Unfortunately "overwrite changes in the working tree" and "discard
local changes" are equally bad.  As it does not say overwrite with
what, we invite the original confusion that triggered these threads
if the reader thought an equally useful but different "overwrites
with result of merging your local changes to the pristine" (similar
to what "checkout -m" does) would happen.

At least, "restore working tree files" without saying "restoring
them to what state?" is much less likely to cause such a confusion.

So perhaps

    git-checkout - Switch branches or restore working tree files

in the headline, and then explain "restore to what state" in the
description?

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Torsten Bögershausen]

On 2015-06-17 18.19, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>> Yes, but "Switch branchs or discard local changes" still does not
>>> describe "git checkout HEAD^^^ -- file.txt" (restore to an old state,
>>> but does not switch branch) or "git checkout -- file.txt" (get from the
>>> index).
>>
>> You are right, especially when file.txt does not have any change
>> relative to HEAD, there is no "discarding" going on.  You are
>> actively introducing a change to an unchanged file by checking
>> contents out of a different revision.
>>
>>> To me, "discard local changes" imply that there will be no uncommited
>>> changes on the files implied in the command after the operation.
>>
>> Yup.
> 
> What was discussed in this thread sounded suspiciously familiar ;-).
> 
> Unfortunately "overwrite changes in the working tree" and "discard
> local changes" are equally bad.  As it does not say overwrite with
> what, we invite the original confusion that triggered these threads
> if the reader thought an equally useful but different "overwrites
> with result of merging your local changes to the pristine" (similar
> to what "checkout -m" does) would happen.
> 
> At least, "restore working tree files" without saying "restoring
> them to what state?" is much less likely to cause such a confusion.
> 
> So perhaps
> 
>     git-checkout - Switch branches or restore working tree files
> 
> in the headline, and then explain "restore to what state" in the
> description?
I'm not sure if the "restore" is always the right thing to describe:
'git checkout <commit> -- <path>'
will "copy" the version from another commit into the workspace.

My v3 will probably use the original line:
git-checkout - Checkout a branch or paths to the working tree

(and improve the description)

git-checkout - Checkout a branch or paths to the working tree

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Matthieu Moy]

Torsten Bögershausen <tboegi@web.de> writes:

> My v3 will probably use the original line:
> git-checkout - Checkout a branch or paths to the working tree

I think mentionning "Switch branch" was a real improvement. For someone
not familiar with the version control vocabulary, "checkout" does not
mean much (just looked in a dictionary, it talks about payment and
leaving a room in a hotel ...). And someone not understanding what
"checkout" means in this context won't be helped much reading the
description and getting "checkout" there.

(Ironically, Junio did almost the same remark when I proposed to
document "git describe" as "Describe ...", but the word "describe" does
not have the ambiguity problem that "checkout" has)

> 'git checkout <commit> -- <path>'
> will "copy" the version from another commit into the workspace.

If <commit> exists, it means that the state of this path existed
somewhere in <path> in the past (well, modulo "git add -p" and other
ways to cheat with history).

So, to me, "restore a previous version" does apply in this case. Perhaps
"restore a recorded state into the worktree" (my favorite up to now I
think).

But as you say, it copies into the workspace, so "copy a previous
version into the workspace" sounds good to me.

Basically, I'm fine with anything starting with "Switch branches or",
but please do change the headline ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Andres G. Aragoneses]

On 17/06/15 19:24, Matthieu Moy wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> My v3 will probably use the original line:
>> git-checkout - Checkout a branch or paths to the working tree
>
> I think mentionning "Switch branch" was a real improvement. For someone
> not familiar with the version control vocabulary, "checkout" does not
> mean much (just looked in a dictionary, it talks about payment and
> leaving a room in a hotel ...). And someone not understanding what
> "checkout" means in this context won't be helped much reading the
> description and getting "checkout" there.
>
> (Ironically, Junio did almost the same remark when I proposed to
> document "git describe" as "Describe ...", but the word "describe" does
> not have the ambiguity problem that "checkout" has)
>
>> 'git checkout <commit> -- <path>'
>> will "copy" the version from another commit into the workspace.
>
> If <commit> exists, it means that the state of this path existed
> somewhere in <path> in the past (well, modulo "git add -p" and other
> ways to cheat with history).
>
> So, to me, "restore a previous version" does apply in this case. Perhaps
> "restore a recorded state into the worktree" (my favorite up to now I
> think).
>
> But as you say, it copies into the workspace, so "copy a previous
> version into the workspace" sounds good to me.
>
> Basically, I'm fine with anything starting with "Switch branches or",
> but please do change the headline ;-).
>

Having read all this thread, I think it's really confusing that:
1) We have this command named "checkout", as Matthieu points out.
2) This command allows different distinct operations (one for when it 
receives a path, other for when it receives a branch, other for when it 
receives a commit...).

So what I would propose is fix the root of the problem: split these 
command in several ones, and mark the "checkout" command as deprecated 
(it would still allow the same functions as before, but it would not be 
documented, and would be announced as deprecated when used).

So then we could have a "git switch <branchname>" for switching to a 
different branch.

Also a "git discard <path>" to discard local changes.

Etcetera.

Comments?

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Junio C Hamano]

"Andres G. Aragoneses" <knocte@gmail.com> writes:

> Comments?

No.

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Torsten Bögershausen <tboegi@web.de> writes:
>
>> My v3 will probably use the original line:
>> git-checkout - Checkout a branch or paths to the working tree
>
> I think mentionning "Switch branch" was a real improvement. For someone
> not familiar with the version control vocabulary, "checkout" does not
> mean much (just looked in a dictionary, it talks about payment and
> leaving a room in a hotel ...). And someone not understanding what
> "checkout" means in this context won't be helped much reading the
> description and getting "checkout" there.

Or, borrow a book from a library, which I think is the closest
analogy for this operation.  But you are right.  It is suboptimal to
explain "checkout" in terms of "checkout" ;-).

> But as you say, it copies into the workspace, so "copy a previous
> version into the workspace" sounds good to me.

I am afraid that "previous" would lead to "Ah, you mean HEAD~1?"
confusion.  In any case, you cannot copy what hasn't yet been
created, "previous" is superfluous.

I think "restore" also by definition has to go back to _some_
existing version, not a future yet-to-be-created one, so "restore to
some previous state" is superfluous; in that sense, I find that
"restore working tree files" may still be the one that makes most
sense, at least to me, among the phrases floated in this thread so
far.

> Basically, I'm fine with anything starting with "Switch branches or",
> but please do change the headline ;-).

Likewise; I agree "switch branches or" part is good.

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Torsten Bögershausen]

On 2015-06-17 21.23, Junio C Hamano wrote:
[]
>> Basically, I'm fine with anything starting with "Switch branches or",
>> but please do change the headline ;-).
> 
> Likewise; I agree "switch branches or" part is good.

How about this:

git-checkout - Switch branches or restore changes to the working tree

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

> On 2015-06-17 21.23, Junio C Hamano wrote:
> []
>>> Basically, I'm fine with anything starting with "Switch branches or",
>>> but please do change the headline ;-).
>> 
>> Likewise; I agree "switch branches or" part is good.
>
> How about this:
>
> git-checkout - Switch branches or restore changes to the working tree

Gahh.  We are NOT restoring CHANGES.  We are restoring the whole
contents to a path.

It is perfectly fine to do this:

	git reset --hard
        git checkout HEAD^ hello.c

There is no changes in hello.c after "reset --hard".

This is what makes it tempting for me to say "check out (an existing
contents to) a working tree file".

Moreover, it does not matter if the target file is changed or not in
the first place, so your added text:

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

that says "restoring modified or deleted is from the index,
replacing is from a tree-ish" is placing a stress on a wrong spot, I
would think.

"Checkout individual files" is to "replace contents with existing
versions, taken either from the index or from a named tree-ish."
That is done in preparation to come up with the suitable contents
for specified paths.

This is a tangent, but on the other hand, "checkout a whole branch"
is to prepare the working tree to be used to modify the specified
branch.  And that is why the word "checkout" makes sense for both
operations.

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Duy Nguyen]

On Thu, Jun 18, 2015 at 2:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> On 2015-06-17 21.23, Junio C Hamano wrote:
>> []
>>>> Basically, I'm fine with anything starting with "Switch branches or",
>>>> but please do change the headline ;-).
>>>
>>> Likewise; I agree "switch branches or" part is good.
>>
>> How about this:
>>
>> git-checkout - Switch branches or restore changes to the working tree
>
> Gahh.  We are NOT restoring CHANGES.  We are restoring the whole
> contents to a path.

"the whole contents" is only true when --patch is not used, I think.
-- 
Duy

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>>> How about this:
>>>
>>> git-checkout - Switch branches or restore changes to the working tree
>>
>> Gahh.  We are NOT restoring CHANGES.  We are restoring the whole
>> contents to a path.
>
> "the whole contents" is only true when --patch is not used, I think.

I've seen that people repeat this "patch is not the whole" and have
ignored that comment; you really need to think if that nitpick adds
anything of value to the description before repeating it.

The "patch" interface of course allows you to pick and choose.  You
have some contents (call it W) in the working tree.  You have
different contents (call it X) somewhere else.  Being able to do
that is the whole point of the feature.

But what is presented you as the choice to pick or ignore?  It is
the difference between W and X.  If you take none from what is
offered, you won't check out anything.  If you take all of them, you
check out the whole of X.  The result is somewhere in between.  

An important point that everybody who repeats "patch is not the
whole" seems to be missing is that it will never be somewhere
between W and Y (the latter of which is different from X).

Now, what is the X in this operation?

It is either what is registered in the index, or in the tree-ish
specified on the command line.

So I'd say that the right mental model to understand the "--patch"
feature is that it allows you to check out the whole contents from
elsewhere; after the command line argument selects from where, i.e.
either from the index or from a tree-ish, you _additionally_ have a
choice to pick which part of that whole to use.  The diff between W
and HEAD or W and index, i.e. "CHANGES", does not play any part of
this selection process.

Re: [PATCH v2] git-checkout.txt: Document "git checkout <pathspec>" better

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> I think "restore" also by definition has to go back to _some_
> existing version, not a future yet-to-be-created one, so "restore to
> some previous state" is superfluous;

That's why I found "restore recorded state" not too bad.

> "restore working tree files"

Not a native, but to me "restore file" sounds like "the file is damaged,
it needs to be restored/repaired".

In any case,

  Switch branches or restore working tree files

is an improvement over the existing to me, so my bikeshedding should not
be used to reject this proposal.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/