Skip to contentSkip to conversation

Documentation/git-checkout: make doc. of checkout <tree-ish> clearer

PATCH In review Christoph Michelbach 3 participants 23 messages Apr 15, 2017 – Apr 25, 2017 1 message not in the archive Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Documentation/git-checkout: make doc. of checkout <tree-ish> clearer

While technically in the documentation, the fact that changes introduced by a checkout <tree-ish> are staged automatically, was not obvious when reading its documentation. It is now specifically pointed out.

---
 Documentation/git-checkout.txt | 7 ++++---

 1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/git-checkout.txt +0 -0
index 8e2c066..cfd7b18 100644
@@ -85,9 +85,10 @@ Omitting <branch> detaches HEAD at the tip of the current branch.

  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.
+ <tree-ish> argument can be used to specify the tree-ish (i.e.
+ commit, tag, or tree) to update the index to for the given paths
+ before updating the working tree accordingly.  Note that this means
+ that the changes this command introduces are staged automatically.
 +
 'git checkout' with <paths> or --patch is used to restore modified or deleted paths to their original contents from the index or replace paths --
2.7.4

#1 lore raw

From: "Christoph Michelbach" <[email protected]>

Quoted 15 lines

While technically in the documentation, the fact that changes introduced by a checkout <tree-ish> are staged automatically, was not obvious when reading its documentation. It is now specifically pointed out.

---
Documentation/git-checkout.txt | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/git-checkout.txt +0 -0
index 8e2c066..cfd7b18 100644
@@ -85,9 +85,10 @@ Omitting <branch> detaches HEAD at the tip of the 
Quoted 6 lines

current branch.
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

Do these lines above actually need reflowing? Their content hasn't changed making it more difficult to spot the significant changes below here.

Quoted 7 lines

- paths before updating the working tree.
+ <tree-ish> argument can be used to specify the tree-ish (i.e.
+ commit, tag, or tree) to update the index to for the given paths
+ before updating the working tree accordingly.

+ Note that this means
+ that the changes this command introduces are staged automatically.

Does this actually capture the intent of the user confusion it's meant to cover? I may have missed the original discussions.

For a clean commit checkout my mental model is not one of anything new being actively staged i.e. different from what was in the commit. I can see that if a particular tree is checkout then it's implicit staging could well be a surprise.

I am in favour of improving the documentation to avoid user surprise

Quoted 6 lines

+
'git checkout' with <paths> or --patch is used to restore modified or deleted paths to their original contents from the index or replace paths

Signature · 1 line
2.7.4

--
Philip

#2 lore raw
Quoted 12 lines · Philip Oakley

From: "Christoph Michelbach" <[email protected]>

Quoted 11 lines

While technically in the documentation, the fact that changes introduced by a checkout <tree-ish> are staged automatically, was not obvious when reading its documentation. It is now specifically pointed out.

---
Documentation/git-checkout.txt | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)
Quoted 14 lines
Quoted 6 lines
Documentation/git-checkout.txt +0 -0
index 8e2c066..cfd7b18 100644
@@ -85,9 +85,10 @@ Omitting <branch> detaches HEAD at the tip of the 
Quoted 6 lines

current branch.
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

Do these lines above actually need reflowing? Their content hasn't changed making it more difficult to spot the significant changes below here.

They're just part of the context of the automatically created patch.

Quoted 4 lines
Quoted 2 lines

+                               Note that this means
+ that the changes this command introduces are staged automatically.

Does this actually capture the intent of the user confusion it's meant to cover? I may have missed the original discussions.

There is no original discussion in this mailing list. I got surprised the command automatically modified my staging area even though I didn't remember to have read it in the corresponding man page. Upon reading the relevant part of the man page again, I noticed that it in fact can be inferred from the half sentence "to update the index for the given paths before updating the working tree." but it isn't pointed out explicitly. Doing so doesn't take a lot more text and can avoid such surprises. So yes, if it's pointed out explicitly, the confusion is removed.

Quoted 2 lines

For a clean commit checkout my mental model is not one of anything new being actively staged i.e. different from what was in the commit.

Note that this is not about something like git checkout 925b29 but about something like git checkout 925b29 src.

Quoted 3 lines

I can see that
if a particular tree is checkout then it's implicit staging could well be a surprise.

And it is actually documented. Just not explicitly mentioned / pointed out.

--
Christoph

#3 lore raw

From: "Christoph Michelbach" <[email protected]>

Quoted 29 lines · Philip Oakley
Quoted 12 lines

From: "Christoph Michelbach" <[email protected]>

Quoted 11 lines

While technically in the documentation, the fact that changes introduced by a checkout <tree-ish> are staged automatically, was not obvious when reading its documentation. It is now specifically pointed out.

---
Documentation/git-checkout.txt | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)
Quoted 15 lines
Quoted 6 lines
Documentation/git-checkout.txt +0 -0
index 8e2c066..cfd7b18 100644
@@ -85,9 +85,10 @@ Omitting <branch> detaches HEAD at the tip of the
Quoted 6 lines

current branch.
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

Do these lines above actually need reflowing? Their content hasn't changed
making it more difficult to spot the significant changes below here.

Quoted 1 line

They're just part of the context of the automatically created patch.

The lines with +/- markings are the actual change lines. It's the lines without them that are the context lines.

It does sound like an accidental reflow where the "(i.e." moved between lines, and the "paths" moved for the following lines.

If reflowing is required it's normal to put it in a separate patch so that each type of change gets its own commit message.

Quoted 22 lines
Quoted 6 lines
Quoted 4 lines

+ Note that this means
+ that the changes this command introduces are staged automatically.

Does this actually capture the intent of the user confusion it's meant to cover? I may have missed the original discussions.

There is no original discussion in this mailing list. I got surprised the command
automatically modified my staging area even though I didn't remember to have read it in
the corresponding man page. Upon reading the relevant part of the man page again, I
noticed that it in fact can be inferred from the half sentence "to update the index for
the given paths before updating the working tree." but it isn't pointed out explicitly.
Doing so doesn't take a lot more text and can avoid such surprises. So yes, if it's
pointed out explicitly, the confusion is removed.

Quoted 7 lines
Quoted 3 lines

For a clean commit checkout my mental model is not one of anything new being
actively staged i.e. different from what was in the commit.

Note that this is not about something like git checkout 925b29 but about something like
git checkout 925b29 src.

Yes, that was the part I was getting at.

The commit message (and the patch context) doesn't quite give enough to see that is is particular to the -

git checkout [-p|--patch] [<tree-ish>] [--] [<paths>…​]invocation.

Quoted 5 lines
Quoted 3 lines

I can see that
if a particular tree is checkout then it's implicit staging could well be a surprise.

And it is actually documented. Just not explicitly mentioned / pointed out.

It maybe worth thinking a bit more about how such a mental model may happen.

To those in the know it sounds obvious that the path content replaces the old path. It's that this implies a *change* within the commit that may not be.

Explaining that it at the heart of the documentation change.

Quoted 3 lines
Quoted 1 line

--

Christoph

Quoted 1 line

--
Philip

#4 lore raw
Quoted 41 lines · Philip Oakley

From: "Christoph Michelbach" <[email protected]>

Quoted 35 lines · Philip Oakley
Quoted 25 lines

From: "Christoph Michelbach" <[email protected]>

Quoted 23 lines

While technically in the documentation, the fact that changes introduced by a checkout <tree-ish> are staged automatically, was
not obvious when reading its documentation. It is now specifically
pointed out.

---
Documentation/git-checkout.txt | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/git-checkout.txt +0 -0
index 8e2c066..cfd7b18 100644
@@ -85,9 +85,10 @@ Omitting <branch> detaches HEAD at the tip of

the
current branch.
from the index file or from a named <tree-ish> (most often a

Quoted 8 lines
Quoted 4 lines

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

Do these lines above actually need reflowing? Their content hasn't changed
making it more difficult to spot the significant changes below here.

Quoted 1 line

They're just part of the context of the automatically created patch.

The lines with +/- markings are the actual change lines. It's the lines
without them that are the context lines.

Quoted 7 lines

It does sound like an accidental reflow where the "(i.e." moved between
lines, and the "paths" moved for the following lines.

If reflowing is required it's normal to put it in a separate patch so that
each type of change gets its own commit message.

Ah, right. I added a missing "to" and changed "to specify a specific".

Quoted 8 lines
Quoted 8 lines
Quoted 4 lines

For a clean commit checkout my mental model is not one of anything new
being
actively staged i.e. different from what was in the commit.

Note that this is not about something like git checkout 925b29 but about
something like
git checkout 925b29 src.

Quoted 7 lines

Yes, that was the part I was getting at.

The commit message (and the patch context) doesn't quite give enough to see
that is is particular to the -

git checkout [-p|--patch] [<tree-ish>] [--] [<paths>…]invocation.

It's: git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>...

The paths aren't optional. Added it to the commit message:

While technically in the documentation, the fact that changes introduced by a git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>... are staged automatically, was not obvious when reading its documentation. It is now specifically pointed out.

Related sentence cleaned up.

---
 Documentation/git-checkout.txt | 7 ++++---

 1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/git- renamed from Documentation/git-checkout.txt +0 -0

checkout.txt
index 8e2c066..cfd7b18 100644

Documentation/git-checkout.txt +0 -0
@@ -85,9 +85,10 @@ Omitting <branch> detaches HEAD at the tip of the

current branch.
        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.
+       <tree-ish> argument can be used to specify the tree-ish (i.e. +       commit, tag, or tree) to update the index to for the given paths +       before updating the working tree accordingly.  Note that this means
+       that the changes this command introduces are staged automatically.
 +
 'git checkout' with <paths> or --patch is used to restore modified or deleted paths to their original contents from the index or replace paths
--
2.7.4

--
Christoph

#5 lore raw

From: "Christoph Michelbach" <[email protected]>

Quoted 63 lines · Philip Oakley
Quoted 41 lines

From: "Christoph Michelbach" <[email protected]>

Quoted 35 lines · Philip Oakley
Quoted 25 lines

From: "Christoph Michelbach" <[email protected]>

Quoted 23 lines

While technically in the documentation, the fact that changes introduced by a checkout <tree-ish> are staged automatically, was
not obvious when reading its documentation. It is now specifically
pointed out.

---
Documentation/git-checkout.txt | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/git-checkout.txt +0 -0
index 8e2c066..cfd7b18 100644
@@ -85,9 +85,10 @@ Omitting <branch> detaches HEAD at the tip of

the
current branch.
from the index file or from a named <tree-ish> (most often a

Quoted 8 lines
Quoted 4 lines

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

Do these lines above actually need reflowing? Their content hasn't changed
making it more difficult to spot the significant changes below here.

Quoted 1 line

They're just part of the context of the automatically created patch.

The lines with +/- markings are the actual change lines. It's the lines
without them that are the context lines.

Quoted 7 lines

It does sound like an accidental reflow where the "(i.e." moved between
lines, and the "paths" moved for the following lines.

If reflowing is required it's normal to put it in a separate patch so that
each type of change gets its own commit message.

Ah, right. I added a missing "to" and changed "to specify a specific".

Quoted 10 lines
Quoted 8 lines
Quoted 4 lines

For a clean commit checkout my mental model is not one of anything new
being
actively staged i.e. different from what was in the commit.

Note that this is not about something like git checkout 925b29 but about
something like
git checkout 925b29 src.

Yes, that was the part I was getting at.

Quoted 8 lines
Quoted 6 lines

The commit message (and the patch context) doesn't quite give enough to see
that is is particular to the -

git checkout [-p|--patch] [<tree-ish>] [--] [<paths>…] invocation.

It's: git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>...

The one I quoted is direct from the Synopsis, which does indicate there are potentially more aspects to resolve, such as the influence of using the [-p|--patch] options.

It maybe that the paragraph / sentence that needs adjusting is;

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

and split it at the "or replace paths" option to pick out your specific case.

Quoted 22 lines

The paths aren't optional. Added it to the commit message:

While technically in the documentation, the fact that changes introduced by a git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>... are staged automatically, was not obvious when reading its documentation. It is now specifically pointed out.

Related sentence cleaned up.

---
Documentation/git-checkout.txt | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/git- renamed from Documentation/git-checkout.txt +0 -0

checkout.txt
index 8e2c066..cfd7b18 100644

Documentation/git-checkout.txt +0 -0
@@ -85,9 +85,10 @@ Omitting <branch> detaches HEAD at the tip of the
Quoted 5 lines

current branch.
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

Ah yes, "specify a specific" does sound a bit tautological.

Quoted 19 lines

- (i.e. commit, tag or tree) to update the index for the given
- paths before updating the working tree.
+ <tree-ish> argument can be used to specify the tree-ish (i.e.
+ commit, tag, or tree) to update the index to for the given paths
+ before updating the working tree accordingly. Note that this means
+ that the changes this command introduces are staged automatically.
+
'git checkout' with <paths> or --patch is used to restore modified or deleted paths to their original contents from the index or replace paths

Signature · 5 lines
2.7.4


--
Christoph
#6 lore raw
Quoted 7 lines · Philip Oakley

From: "Christoph Michelbach" <[email protected]>

Quoted 1 line

It's: git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>...

The one I quoted is direct from the Synopsis, which does indicate there are
potentially more aspects to resolve, such as the influence of using the
[-p|--patch] options.

Oh, you are right. I didn't even notice the one in the synopsis doesn't match the one further down. The one in the synopsis is wrong because after removing the optional parameters, it's the same as the first one in the synopsis, yet we expect very different behavior from them.

Quoted 11 lines

It maybe that the paragraph / sentence that needs adjusting is;

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

and split it at the "or replace paths" option to pick out your specific
case.

This one is confusing, too: Paths can lead to folders, yet folders whose contents have been modified are not restored to their original contents when executing that command. Only files are.

After reading the documentation and having never used the command before, one would expect

#!/bin/bash
rm -Rf repo
mkdir repo
cd repo
git init &> /dev/null
mkdir folder
echo a > folder/a
git add -A
git commit -m "Commit 1." &> /dev/null
echo b > folder/b
git add -A
git commit -m "Commit 2." &> /dev/null
echo c > folder/c
git add -A
git commit -m "Commit 3." &> /dev/null
git checkout git log --pretty=format:%H | tail -1 folder ls folder

to print a. However, it prints a b c because all of the files inside folder which have been modified or deleted since (here: none) are reset to their original state after the first commit, but folder itself isn't. Yet, the only path which was passed to the command in question is folder.

In my opinion, this command needs improved documentation (and the synopsis needs to be fixed).

--
Christoph

#7 lore raw

From: "Christoph Michelbach" <[email protected]>

Quoted 52 lines · Philip Oakley
Quoted 7 lines

From: "Christoph Michelbach" <[email protected]>

Quoted 1 line

It's: git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>...

The one I quoted is direct from the Synopsis, which does indicate there are
potentially more aspects to resolve, such as the influence of using the
[-p|--patch] options.

Oh, you are right. I didn't even notice the one in the synopsis doesn't match the one further down. The one in the synopsis is wrong because after removing the optional parameters, it's the same as the first one in the synopsis, yet we expect very different behavior from them.

Quoted 11 lines

It maybe that the paragraph / sentence that needs adjusting is;

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

and split it at the "or replace paths" option to pick out your specific
case.

This one is confusing, too: Paths can lead to folders, yet folders whose contents have been modified are not restored to their original contents when executing that command. Only files are.

After reading the documentation and having never used the command before, one would expect

   #!/bin/bash
   rm -Rf repo
   mkdir repo
   cd repo
   git init &> /dev/null
   mkdir folder
   echo a > folder/a
   git add -A
   git commit -m "Commit 1." &> /dev/null
   echo b > folder/b
   git add -A
   git commit -m "Commit 2." &> /dev/null
   echo c > folder/c
   git add -A
   git commit -m "Commit 3." &> /dev/null
   git checkout `git log --pretty=format:%H | tail -1` folder
   ls folder
Quoted 9 lines

to print a. However, it prints a b c because all of the files inside folder which have been modified or deleted since (here: none) are reset to their original state after the first commit, but folder itself isn't. Yet, the only path which was passed to the command in question is folder.

In my opinion, this command needs improved documentation (and the synopsis needs to be fixed).

I think this example is of a different kind of misunderstanding.

We are at commit 3, with a b c in the working directory and index, and then we ask to checkout certain specific files from the first commit 1, which contains the file a. That old file is extracted and replaces the file from commit 3.

As the file a is identical there is no change and we still have the original b and c files from commit c.

I'd guess that the misunderstanding is that you maybe thought that the whole directory would be reset to it's old state and the files b and c deleted, rather than just the named files present in that old commit being extracted. If we'd created and added a file d just before the checkout, what should have happened to d, and why?

Quoted 1 line

Comming back to the mental model issue with the implicit staging of checked out paths, I suspect this is a because we have both a snapshot and a diff based mental model. Normally the distinction is natural. We have the snapshot from the last commit (or branch checkout) in the index, and then we have the two steps for additional changes we personally made, and then added added, that determine the diff(s).

However in this (git checkout <treeish> -- <paths>) case we don't get that two step option. There is IIUC no 'git copyout <treeish> -- <paths>' which simply copies older files into the current worktree without adding it to the index/staging area.

The confounding of the, both optional, [--patch] and [<paths>] in the same description doesn't make it any easier. Splitting their synopses and descriptions may be the way forward.

I also haven't used the --patch option directly so there may be more issues beneath the surface.

Quoted 4 lines

--
Christoph

--
Philip

A message that is not in the archive (sent off-list or not yet received): [email protected]

#9 lore raw

I've added back the list, as it was accidentally dropped.

From: "Christoph Michelbach" <[email protected]>

Quoted 103 lines · Philip Oakley
Quoted 86 lines

From: "Christoph Michelbach" <[email protected]>

Quoted 68 lines · Philip Oakley
Quoted 10 lines

From: "Christoph Michelbach" <[email protected]>

Quoted 2 lines

It's: git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>...

The one I quoted is direct from the Synopsis, which does indicate there are
potentially more aspects to resolve, such as the influence of using
the
[-p|--patch] options.

Oh, you are right. I didn't even notice the one in the synopsis doesn't
match the one further down. The one in the synopsis is wrong because after removing the optional parameters, it's the same as the first one
in the synopsis, yet we expect very different behavior from them.

Quoted 11 lines

It maybe that the paragraph / sentence that needs adjusting is;

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

Quoted 3 lines

and split it at the "or replace paths" option to pick out your specific
case.

This one is confusing, too: Paths can lead to folders, yet folders whose
contents have been modified are not restored to their original contents
when executing that command. Only files are.

After reading the documentation and having never used the command before, one would expect

#!/bin/bash
rm -Rf repo
mkdir repo
cd repo
git init &> /dev/null
mkdir folder
echo a > folder/a
git add -A
git commit -m "Commit 1." &> /dev/null
echo b > folder/b
git add -A
git commit -m "Commit 2." &> /dev/null
echo c > folder/c
git add -A
git commit -m "Commit 3." &> /dev/null
git checkout git log --pretty=format:%H | tail -1 folder ls folder

to print a. However, it prints a b c because all of the files inside
folder which have been modified or deleted since (here: none) are reset to their original state after the first commit, but folder itself isn't. Yet, the only path which was passed to the command in question is folder.

Quoted 3 lines

In my opinion, this command needs improved documentation (and the synopsis needs to be fixed).

I think this example is of a different kind of misunderstanding.

We are at commit 3, with a b c in the working directory and index, and then
we ask to checkout certain specific files from the first commit 1, which
contains the file a. That old file is extracted and replaces the file from
commit 3.

As the file a is identical there is no change and we still have the original
b and c files from commit c.

Quoted 9 lines

I'd guess that the misunderstanding is that you maybe thought that the whole
directory would be reset to it's old state and the files b and c deleted,
rather than just the named files present in that old commit being extracted.
If we'd created and added a file d just before the checkout, what should
have happened to d, and why?

I understand what the command does. It behaves perfectly as I expected it to. I did not find this script but wrote it to demonstrate that what the documentation says is different from how it behaves after having read what the documentation says it should do and noticing that that's not how I expected it to work from experience.

Quoted 6 lines

What it really does is to copy all files described by the given paths from the given tree-ish to the working directory. Or at least that's my expectation of what it does.

The documentation, however, says that the given paths are *restored*. This is different.

I don't see that difference in the phrase *restored*, compared to your 'copy all files described by the given paths'. Could you explain a little more?

Quoted 59 lines

To answer your question: Nothing should happen to the file d. I stated what I genuinely believe to be true above: "What it really does is to copy all files described by the given paths from the given tree-ish to the working directory." If we take this and apply it, we see that d is not touched because there is nothing in the given tree-ish that could override it because d is not in the index.

If we, however, take the existing documentation and apply it, d is removed if and only if a path leading to d (like d if d is in the root of the repo or folder or folder/d if it is in the folder folder) is passed as an argument.

Quoted 11 lines

Comming back to the mental model issue with the implicit staging of checked
out paths, I suspect this is a because we have both a snapshot and a diff
based mental model. Normally the distinction is natural. We have the snapshot from the last commit (or branch checkout) in the index, and then we
have the two steps for additional changes we personally made, and then added
added, that determine the diff(s).

Quoted 7 lines

However in this (git checkout <treeish> -- <paths>) case we don't get that
two step option. There is IIUC no 'git copyout <treeish> -- <paths>' which
simply copies older files into the current worktree without adding it to the
index/staging area.

Well, technically we can git reset to that point, then git checkout- index where the index is already up-to-date with the state of the working tree we want regarding the files we care about, and then git reset back, but I don't think there is a single command for that, either.

Quoted 8 lines

The confounding of the, both optional, [--patch] and [<paths>] in the same
description doesn't make it any easier. Splitting their synopses and descriptions may be the way forward.

I also haven't used the --patch option directly so there may be more issues
beneath the surface.

Yeah, definitely. There should be 2 separate entries for this.

I think someone thought it was a good idea to make <pathspec>... optional in the synopsis because git checkout behaves in that special way if a patch *or* paths are given. But then, of course, with both - p|--patch and <pathspec>... optional, the command is the same as the first variation, just with optional parameters -- but the documentation (correctly) says those variations should behave very differently from each other.

Quoted 2 lines

I don't see how this can be avoided without having 2 separate entries for those cases.

true.

--
Philip

#10 lore raw
Quoted 14 lines · Philip Oakley
Quoted 14 lines
Quoted 3 lines

If we'd created and added a file d just before the checkout, what should
have happened to d, and why?

I understand what the command does. It behaves perfectly as I expected it to. I did not find this script but wrote it to demonstrate that what the documentation says is different from how it behaves after having read what the documentation says it should do and noticing that that's not how I expected it to work from experience.

What it really does is to copy all files described by the given paths from the given tree-ish to the working directory. Or at least that's my expectation of what it does.

Quoted 6 lines
Quoted 2 lines

The documentation, however, says that the given paths are *restored*. This is different.

I don't see that difference in the phrase *restored*, compared to your 'copy all files described by the given paths'. Could you explain a little more?

I am obviously not Christoph, and I was the one that defined how "checkout <tree> -- <pathspec>" should work, but when you say "restore" (which is not what I wrote ;-)) it is fair to expect lack of 'd' could also be "restored", in addition to path that was in the directory.

Obviously, "grab all paths that match <pathspec> out of <tree>, add them to the index and copy them out to the working tree" will never be able to _restore_ the lack of 'd', even it may match the <pathspec> being used to do this checkout, by removing it from the current index and the working tree.

#11 lore raw
Quoted 32 lines · Junio C Hamano
Quoted 23 lines · Philip Oakley
Quoted 23 lines
Quoted 7 lines

If we'd created and added a file d just before the checkout, what
should
have happened to d, and why?

I understand what the command does. It behaves perfectly as I expected
it to. I did not find this script but wrote it to demonstrate that what
the documentation says is different from how it behaves after having
read what the documentation says it should do and noticing that that's
not how I expected it to work from experience.

What it really does is to copy all files described by the given paths
from the given tree-ish to the working directory. Or at least that's my
expectation of what it does.

Quoted 7 lines
Quoted 3 lines

The documentation, however, says that the given paths are *restored*.
This is different.

I don't see that difference in the phrase *restored*, compared to your
'copy all files described by the given paths'. Could you explain a little more?

Quoted 5 lines

I am obviously not Christoph, and I was the one that defined how "checkout <tree> -- <pathspec>" should work, but when you say "restore" (which is not what I wrote ;-)) it is fair to expect lack of 'd' could also be "restored", in addition to path that was in the directory.

Yes, you didn't write "restore" but you wrote: "It updates the named paths in the working tree from the index file or from a named <tree-ish> (most often a commit)."

I suppose that from reading this, some people will assume that d is gone after the command was executed because the folder to which the given path leads is updated "in the working tree from the index file or from a named <tree-ish>", and others will not be sure what happens. But I doubt that a lot of people guess the behavior correctly and are sure about them being correct.

Note that for the first group of people, it doesn't matter that git doesn't track folders. You can restore a folder "in the working tree from the index file or from a named <tree-ish>" without tracking folders because there only is one property of a folder git cares about, apart from it itself being able to access files in that folder: Its name. And it can get that name from the paths of the files in that folder. If there are no files in that folder, there isn't a contradiction, either, because then there is no folder in the index or tree-ish which can possibly be restored.

Quoted 5 lines

Obviously, "grab all paths that match <pathspec> out of <tree>, add them to the index and copy them out to the working tree" will never be able to _restore_ the lack of 'd', even it may match the <pathspec> being used to do this checkout, by removing it from the current index and the working tree.

Exactly. "grab all paths that match <pathspec> out of <tree>, add them to the index and copy them out to the working tree" is a more accurate description of what the command does (but it might need some rewording ;-) ).

#12 lore raw
Quoted 8 lines · Christoph Michelbach
Quoted 5 lines · Junio C Hamano

Obviously, "grab all paths that match <pathspec> out of <tree>, add them to the index and copy them out to the working tree" will never be able to _restore_ the lack of 'd', even it may match the <pathspec> being used to do this checkout, by removing it from the current index and the working tree.

Quoted 4 lines

Exactly. "grab all paths that match <pathspec> out of <tree>, add them to the index and copy them out to the working tree" is a more accurate description of what the command does (but it might need some rewording ;-) ).

Of course it is accurate, as that is how I would write it, not "restore", if I were doing the documentation.

Care to send in a patch to update the documentation?

#13 lore raw
Quoted 20 lines · Junio C Hamano
Quoted 11 lines · Christoph Michelbach
Quoted 8 lines · Junio C Hamano

Obviously, "grab all paths that match <pathspec> out of <tree>, add
them to the index and copy them out to the working tree" will never
be able to _restore_ the lack of 'd', even it may match the <pathspec> being used to do this checkout, by removing it from the current index and the working tree.

Quoted 7 lines

Exactly. "grab all paths that match <pathspec> out of <tree>, add them
to the index and copy them out to the working tree" is a more accurate
description of what the command does (but it might need some rewording
;-) ).

Quoted 4 lines

Of course it is accurate, as that is how I would write it, not "restore", if I were doing the documentation.

Care to send in a patch to update the documentation?

From 10c362b0632255f90e0975fb6656feedca5fd407 Mon Sep 17 00:00:00 2001
From: Christoph Michelbach <[email protected]>
Date: Sat, 22 Apr 2017 18:49:57 +0200
Subject: [PATCH] Doc./git-checkout: correct doc. of checkout <pathspec>...

The previous documentation states that the named paths are updated both in the index and the working tree. This is not correct as those paths can point to folders which are not updated to what they are in the given tree-ish. Rather, the files pointed to by the pathspecs are copied from the tree-ish to the index and working tree One difference being that one can name paths which are not present in the working tree and another being that only files which are already present in the given tree-ish are affected.

Furthermore, the changed text is intended to be more intelligible as it is written in a way which is easy to follow, rather than describing the technical process in order of execution.

A hint alerting the users that changes introduced by this command when naming a tree-ish are automatically staged has been introduced.

---
 Documentation/git-checkout.txt | 16 +++++++++-------

 1 file changed, 9 insertions(+), 7 deletions(-)
Documentation/git- renamed from Documentation/git-checkout.txt +0 -0

checkout.txt
index 8e2c066..f74f237 100644

Documentation/git-checkout.txt +0 -0
@@ -81,13 +81,15 @@ Omitting <branch> detaches HEAD at the tip of the

current branch.
 'git checkout' [-p|--patch] [<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.
+ switch branches.  It copies the files matching the pathspecs in
+ <tree-is> (i.e. a commit, tag, or tree) to the index and
+ subsequently to the working directory, overwriting changes
+ (including deletion of files) in those files.  In this case,
the
+ -b and --track options are meaningless and giving either of
them
+ results in an error.  Note that because the index is updated,
the
+ changes introduced by this command are automatically
staged.  If
+ no tree-ish is provided, the current index is used and remains
+ unchanged, modifying only the working directory.
 +
 'git checkout' with <paths> or --patch is used to restore modified or deleted paths to their original contents from the index or replace paths
--
2.7.4

#14 lore raw
Quoted 1 line · Christoph Michelbach
Quoted 1 line

Care to send in a patch to update the documentation?

Thanks. The attached patch text appears linewrapped, but I think I'll be able to salvage, so forgetting about the formatting, here are some comments.

Quoted 10 lines

From: Christoph Michelbach <[email protected]>
Date: Sat, 22 Apr 2017 18:49:57 +0200
Subject: [PATCH] Doc./git-checkout: correct doc. of checkout <pathspec>...

The previous documentation states that the named paths are updated both in the index and the working tree. This is not correct as those paths can point to folders which are not updated to what they are in the given tree-ish. Rather, the files pointed to by the pathspecs are copied from the

s/pointed to by/that match/

Quoted 1 line

tree-ish to the index and working tree One difference being

Missing full-stop after "tree".

Quoted 2 lines

that one can name paths which are not present in the working tree ...

That one is not a difference, I would think. If your current index and working tree lack F, you give pathspec that match F, and the tree-ish has that path F, that named path _is_ updated both in the index and in the working tree.

Quoted 2 lines

... and another being that only files which are already present in the given tree-ish are affected.

This one indeed was missing piece of information.

Quoted 15 lines

---
 Documentation/git-checkout.txt | 16 +++++++++-------

 1 file changed, 9 insertions(+), 7 deletions(-)
Documentation/git- renamed from Documentation/git-checkout.txt +0 -0

checkout.txt
index 8e2c066..f74f237 100644

Documentation/git-checkout.txt +0 -0
@@ -81,13 +81,15 @@ Omitting <branch> detaches HEAD at the tip of the

current branch.
 'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::

  When <paths> or --patch are given, 'git checkout' does *not*

Documentation/git-checkout.txt

Quoted 2 lines
Documentation/git-checkout.txt +2 -0
+	switch branches.  It copies the files matching the pathspecs in
+	<tree-is> (i.e. a commit, tag, or tree) to the index and

<tree-ish> is misspelled here. Drop (i.e. ...) as (1) it is not correct (a tag may not point at a tree-ish) and (2) "checkout" is not a place to learn what a tree-ish is (glossary is).

Documentation/git-checkout.txt

Quoted 1 line
Documentation/git-checkout.txt +1 -0
+	subsequently to the working directory, overwriting changes

Do we need to say "subsequently" when we are aiming to be more intelligible by not describing the order of execution? The end result is that the blobs named by the pathspecs from the tree-ish are checked out to the index and to the working tree at the same time.

Documentation/git-checkout.txt

Quoted 2 lines

+... Note that because the index is updated, the

Documentation/git-checkout.txt +1 -0
+	changes introduced by this command are automatically staged.

This is redundant and unnecessary, I would say. If you absolutely need to say this, at least drop "automatically". There is nothing automatic about it. The user is asking to checkout the named blobs out of the tree-ish to the index and to the working tree, and Git is merely doing as it was told.

In addition the description is fuzzy; what are "changes introduced by this command" relative to? If you did "checkout HEAD path" after editing path in the working tree, is "reverting my edit" the "changes introduced by this command"? If you did "checkout HEAD path" after editing path, "git add path" and then editing path further, what are the "changes introduced by this command"?

#15 lore raw
Quoted 6 lines · Junio C Hamano
Quoted 5 lines · Christoph Michelbach

From: Christoph Michelbach <[email protected]>
Date: Sat, 22 Apr 2017 18:49:57 +0200
Subject: [PATCH] Doc./git-checkout: correct doc. of checkout <pathspec>...

Quoted 6 lines
Quoted 5 lines

The previous documentation states that the named paths are updated both in the index and the working tree. This is not correct as those paths can point to folders which are not updated to what they are in the given tree-ish. Rather, the files pointed to by the pathspecs are copied from the

s/pointed to by/that match/

Ok.

Quoted 3 lines
Quoted 2 lines

tree-ish to the index and working tree One difference being

Missing full-stop after "tree".

Oops.

Quoted 7 lines
Quoted 3 lines

that one can name paths which are not present in the working tree ...

That one is not a difference, I would think.  If your current index and working tree lack F, you give pathspec that match F, and the tree-ish has that path F, that named path _is_ updated both in the index and in the working tree.

I don't think it's inherently obvious that you can name paths which don't occur in the working tree if it's stated that those are updated. For something to be updated, I suppose it has to exist in the first place. But I don't really want to discuss the commit message as everyone even reading it already knows how the command works so I just removed it.

From 24a5ca5affbe8a09413b72ceb3cb50ae2fe9fd2b Mon Sep 17 00:00:00 2001
From: Christoph Michelbach <[email protected]>
Date: Sat, 22 Apr 2017 18:49:57 +0200
Subject: [PATCH] Doc./git-checkout: correct doc. of checkout <pathspec>...

The previous documentation states that the named paths are updated both in the index and the working tree. This is not correct as those paths can point to folders which are not updated to what they are in the given tree-ish. Rather, the files which match the pathspecs are copied from the tree-ish to the index and working tree. The difference being that only files which are already present in the given tree-ish are affected.

Furthermore, the changed text is intended to be more intelligible as it is written in a way which is easy to follow, rather than describing the technical process in order of execution.

A hint alerting the users that changes introduced by this command when naming a tree-ish are automatically staged has been introduced.

---
 Documentation/git-checkout.txt | 16 +++++++++-------

 1 file changed, 9 insertions(+), 7 deletions(-)
Documentation/git-checkout.txt +0 -0
index 8e2c066..306b9d2 100644
@@ -81,13 +81,15 @@ Omitting <branch> detaches HEAD at the tip of the

current branch.
 'git checkout' [-p|--patch] [<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.
+ switch branches.  It copies the files matching the pathspecs in
+ <tree-is> (i.e. a commit, tag, or tree) to the index and
+ subsequently to the working tree, overwriting changes
+ (including deletion of files) in those files.  In this case, the
+ -b and --track options are meaningless and giving either of them
+ results in an error.  Note that because the index is updated, the
+ changes introduced by this command are automatically staged.  If
+ no tree-ish is provided, the current index is used and remains
+ unchanged, modifying only 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 --
2.7.4

#16 lore raw
In reply to Junio C Hamano

I'm sorry, somehow my email client deleted half of your message when I saved it when replying. I only noticed this when wanting to delete your email and noticed that it was pretty long.

--------------------------------------------------------------------------------

Quoted 11 lines · Junio C Hamano
Quoted 10 lines · Christoph Michelbach

From: Christoph Michelbach <[email protected]>
Date: Sat, 22 Apr 2017 18:49:57 +0200
Subject: [PATCH] Doc./git-checkout: correct doc. of checkout <pathspec>...

The previous documentation states that the named paths are updated both in the index and the working tree. This is not correct as those paths can point to folders which are not updated to what they are in the given tree-ish. Rather, the files pointed to by the pathspecs are copied from the

Quoted 1 line

s/pointed to by/that match/

Ok.

Quoted 3 lines
Quoted 2 lines


tree-ish to the index and working tree One difference being

Missing full-stop after "tree".

Oops.

Quoted 7 lines
Quoted 3 lines


that one can name paths which are not present in the working tree ...

That one is not a difference, I would think.  If your current index and working tree lack F, you give pathspec that match F, and the tree-ish has that path F, that named path _is_ updated both in the index and in the working tree.

I don't think it's inherently obvious that you can name paths which don't occur in the working tree if it's stated that those are updated. For something to be updated, I suppose it has to exist in the first place. But I don't really want to discuss the commit message as everyone even reading it already knows how the command works so I just removed it.

Quoted 6 lines

Review

Quoted 3 lines

  When <paths> or --patch are given, 'git checkout' does *not*

(quoted diff) +2 -0
+	switch branches.  It copies the files matching the pathspecs in
+	<tree-is> (i.e. a commit, tag, or tree) to the index and

<tree-ish> is misspelled here.  Drop (i.e. ...) as (1) it is not correct (a tag may not point at a tree-ish) and (2) "checkout" is not a place to learn what a tree-ish is (glossary is).

Done.

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +1 -0
+	subsequently to the working directory, overwriting changes

Do we need to say "subsequently" when we are aiming to be more intelligible by not describing the order of execution?  The end result is that the blobs named by the pathspecs from the tree-ish are checked out to the index and to the working tree at the same time.

I agree. Done.

Quoted 9 lines

Review

Quoted 4 lines

+... Note that because the index is updated, the

(quoted diff) +1 -0
+	changes introduced by this command are automatically staged.

This is redundant and unnecessary, I would say.  If you absolutely need to say this, at least drop "automatically".  There is nothing automatic about it.  The user is asking to checkout the named blobs out of the tree-ish to the index and to the working tree, and Git is merely doing as it was told.

Done.

Quoted 4 lines

In addition the description is fuzzy; what are "changes introduced by this command" relative to?  If you did "checkout HEAD path" after editing path in the working tree, is "reverting my edit" the "changes introduced by this command"?

All of those no changes which could possibly be staged are immediately staged. All statements about all elements of the empty set are true.

Quoted 3 lines

If you did "checkout HEAD
path" after editing path, "git add path" and then editing path further, what are the "changes introduced by this command"?

The command can't introduce any further changes just because you edit files after it has already terminated execution.

From fe0d1298cf4de841af994f4d9f72d49e25edea00 Mon Sep 17 00:00:00 2001
From: Christoph Michelbach <[email protected]>
Date: Sat, 22 Apr 2017 18:49:57 +0200
Subject: [PATCH] Doc./git-checkout: correct doc. of checkout <pathspec>...

The previous documentation states that the named paths are updated both in the index and the working tree. This is not correct as those paths can point to folders which are not updated to what they are in the given tree-ish. Rather, the files which match the pathspecs are copied from the tree-ish to the index and working tree. The difference being that only files which are already present in the given tree-ish are affected.

Furthermore, the changed text is intended to be more intelligible as it is written in a way which is easy to follow, rather than describing the technical process in order of execution.

A hint alerting the users that changes introduced by this command when naming a tree-ish are automatically staged has been introduced.

---
 Documentation/git-checkout.txt | 15 ++++++++-------

 1 file changed, 8 insertions(+), 7 deletions(-)
Documentation/git-checkout.txt +0 -0
index 8e2c066..ea3b4df 100644
@@ -81,13 +81,14 @@ Omitting <branch> detaches HEAD at the tip of the current

branch.
 'git checkout' [-p|--patch] [<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.
+ switch branches.  It copies the files matching the pathspecs in
+ <tree-ish> to the index and to the working tree, overwriting
+ changes (including deletion of files) in those files.  In this
+ case, the -b and --track options are meaningless and giving either
+ of them results in an error.  Note that because the index is updated,
+ the changes introduced by this command are staged.  If no tree-ish is
+ provided, the current index is used and remains unchanged, modifying
+ only 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 --
2.7.4

#17 lore raw
Quoted 3 lines · Christoph Michelbach

I'm sorry, somehow my email client deleted half of your message when I saved it when replying. I only noticed this when wanting to delete your email and noticed that it was pretty long.

Please separate your discussion from the patch proper. Check Documentation/SubmittingPatches and send a proper patch like other people do.

Quoted 4 lines

From fe0d1298cf4de841af994f4d9f72d49e25edea00 Mon Sep 17 00:00:00 2001
From: Christoph Michelbach <[email protected]>
Date: Sat, 22 Apr 2017 18:49:57 +0200
Subject: [PATCH] Doc./git-checkout: correct doc. of checkout <pathspec>...

These we take from the e-mail header. You usually remove them from the body of the message (and move the Subject: to e-mail subject), hence

Quoted 1 line

The previous documentation states that the named paths are

this line will become the first line in the body of the message.

Quoted 3 lines

A hint alerting the users that changes introduced by this command when naming a tree-ish are automatically staged has been introduced.

We are still saying automatically here?

Quoted 4 lines

---
 Documentation/git-checkout.txt | 15 ++++++++-------

 1 file changed, 8 insertions(+), 7 deletions(-)

This is not limited to your message, but from time to time, I see messages with SP substituted with non-breaking space like the above two lines (and they appear in the patch text). I can still read and comment on the patch, but it is unusuable as an input to "git am" to be applied. I wonder where these NBSP come from---perhaps some commmon MUAs corrupt text messages like this?

Quoted 10 lines
Documentation/git-checkout.txt +0 -0
index 8e2c066..ea3b4df 100644
@@ -81,13 +81,14 @@ Omitting <branch> detaches HEAD at the tip of the current

branch.
 'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::

  When <paths> or --patch are given, 'git checkout' does *not*

Documentation/git-checkout.txt

Quoted 8 lines
Documentation/git-checkout.txt +1 -7
-	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.
+	switch branches.  It copies the files matching the pathspecs in

I am debating myself if rephrasing "in <tree-ish>" to "from <tree-ish>" makes the result clearer to understand. When we say "Copy files from T to I and W", it would be obvious that what does not exist in T will not be copied.

I also wonder "If no-tree-ish is provided" at the end of this paragraph you have is a bit too far from the above primary point of the description, because you have an unrelated "In this case, -b/-t...", in between.

The blobs matching the pathspecs are checked out from the index to the working tree. Optionally, when <tree-ish> is given, the blobs matching the pathspecs from the <tree-ish> is copied to the index before that happens.

is what I would want to say, but obviously that does not describe what happens in the chronological order, so it is the most clear description for people who understand what is written, but it will take two reading until the reader gets to that stage X-<.

Perhaps the unrelated "In this case, the -b..." should go first; it is part of "does *not* switch branches". Also even with your patch, it starts with "X is not Y" and does not clearly say "X is Z".

When <paths> or --patch ar given, 'git checkout' does *not* switch branches (giving the -b and --track options will cause an error, as they are meaningless). It checks out paths out of the <tree-ish> (if given) and the index to the to working tree. When an optional <tree-ish> is given blobs in the <tree-ish> that match <pathspec> are copied to the index. The blobs that match <pathspec> are then copied from the index to the working tree, overwriting what is in (or missing from) the working tree.

May be an improvement (i.e. say what Z is: checking out paths from tree-ish and/or index to the working tree). By explicitly phrasing that <tree-ish>, from which the index is updated, is optional, it is clear that without <tree-ish> there is no update to the index. "missing from" covers two cases: (1) the user removed the file from the working tree and <tree-ish>, e.g. HEAD, has the file, hence removed one is resurrected; (2) the user didn't touch the file and HEAD didn't have it, but by checking out from <tree-ish> that has the file, the user added that new file to the set of files the user is working with.

Hmm?

#18 lore raw

I give up. Bye.

Quoted 110 lines · Junio C Hamano
Quoted 6 lines · Christoph Michelbach

I'm sorry, somehow my email client deleted half of your message when I saved it
when replying. I only noticed this when wanting to delete your email and noticed
that it was pretty long.

Please separate your discussion from the patch proper.  Check Documentation/SubmittingPatches and send a proper patch like other people do.

Quoted 5 lines

From fe0d1298cf4de841af994f4d9f72d49e25edea00 Mon Sep 17 00:00:00 2001
From: Christoph Michelbach <[email protected]>
Date: Sat, 22 Apr 2017 18:49:57 +0200
Subject: [PATCH] Doc./git-checkout: correct doc. of checkout <pathspec>...

These we take from the e-mail header.  You usually remove them from the body of the message (and move the Subject: to e-mail subject), hence

Quoted 2 lines

The previous documentation states that the named paths are

this line will become the first line in the body of the message.

Quoted 4 lines

A hint alerting the users that changes introduced by this command when naming a tree-ish are automatically staged has been introduced.

We are still saying automatically here?

Quoted 5 lines

---
 Documentation/git-checkout.txt | 15 ++++++++-------

 1 file changed, 8 insertions(+), 7 deletions(-)

This is not limited to your message, but from time to time, I see messages with SP substituted with non-breaking space like the above two lines (and they appear in the patch text).  I can still read and comment on the patch, but it is unusuable as an input to "git am" to be applied.  I wonder where these NBSP come from---perhaps some commmon MUAs corrupt text messages like this?

Quoted 12 lines
Documentation/git-checkout.txt +0 -0
index 8e2c066..ea3b4df 100644
@@ -81,13 +81,14 @@ Omitting <branch> detaches HEAD at the tip of the

current
branch.
 'git checkout' [-p|--patch] [<tree-ish>] [--] <pathspec>...::

  When <paths> or --patch are given, 'git checkout' does *not*

Documentation/git-checkout.txt

Quoted 8 lines
Documentation/git-checkout.txt +1 -7
-	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.
+	switch branches.  It copies the files matching the pathspecs in

I am debating myself if rephrasing "in <tree-ish>" to "from <tree-ish>" makes the result clearer to understand.  When we say "Copy files from T to I and W", it would be obvious that what does not exist in T will not be copied.

I also wonder "If no-tree-ish is provided" at the end of this paragraph you have is a bit too far from the above primary point of the description, because you have an unrelated "In this case, -b/-t...", in between.

The blobs matching the pathspecs are checked out from the index to the working tree.  Optionally, when <tree-ish> is given, the blobs matching the pathspecs from the <tree-ish> is copied to the index before that happens.

is what I would want to say, but obviously that does not describe what happens in the chronological order, so it is the most clear description for people who understand what is written, but it will take two reading until the reader gets to that stage X-<.

Perhaps the unrelated "In this case, the -b..." should go first; it is part of "does *not* switch branches".  Also even with your patch, it starts with "X is not Y" and does not clearly say "X is Z".

When <paths> or --patch ar given, 'git checkout' does *not* switch branches (giving the -b and --track options will cause an error, as they are meaningless).  It checks out paths out of the <tree-ish> (if given) and the index to the to working tree.  When an optional <tree-ish> is given blobs in the <tree-ish> that match <pathspec> are copied to the index.  The blobs that match <pathspec> are then copied from the index to the working tree, overwriting what is in (or missing from) the working tree.

May be an improvement (i.e. say what Z is: checking out paths from tree-ish and/or index to the working tree).  By explicitly phrasing that <tree-ish>, from which the index is updated, is optional, it is clear that without <tree-ish> there is no update to the index. "missing from" covers two cases: (1) the user removed the file from the working tree and <tree-ish>, e.g. HEAD, has the file, hence removed one is resurrected; (2) the user didn't touch the file and HEAD didn't have it, but by checking out from <tree-ish> that has the file, the user added that new file to the set of files the user is working with.

Hmm?

#19 lore raw
In reply to Junio C Hamano

From: "Junio C Hamano" <[email protected]> Sent: Tuesday, April 18, 2017 1:31 AM

Quoted 29 lines
Quoted 14 lines · Philip Oakley
Quoted 14 lines
Quoted 3 lines

If we'd created and added a file d just before the checkout, what should
have happened to d, and why?

I understand what the command does. It behaves perfectly as I expected it to. I did not find this script but wrote it to demonstrate that what the documentation says is different from how it behaves after having read what the documentation says it should do and noticing that that's not how I expected it to work from experience.

What it really does is to copy all files described by the given paths from the given tree-ish to the working directory. Or at least that's my expectation of what it does.

Quoted 6 lines
Quoted 2 lines

The documentation, however, says that the given paths are *restored*. This is different.

I don't see that difference in the phrase *restored*, compared to your 'copy all files described by the given paths'. Could you explain a little more?

I am obviously not Christoph, and I was the one that defined how "checkout <tree> -- <pathspec>" should work, but when you say "restore" (which is not what I wrote ;-)) it is fair to expect lack of 'd' could also be "restored", in addition to path that was in the directory.

Quoted 6 lines

Obviously, "grab all paths that match <pathspec> out of <tree>, add them to the index and copy them out to the working tree" will never be able to _restore_ the lack of 'd', even it may match the <pathspec> being used to do this checkout, by removing it from the current index and the working tree.

My attempt at asking about an additional file 'd' ended up being a bit of a red herring as it went off at a tangent.

Hopefully Christoph will hang in to help clarify the original issue. I think I'm getting a sense of the potential confusion between the different messages Git sends out, and which one is the right one here. On the one hand Git promotes the two step staging area approach, but on the other hand the checkout of a file suggests IIUC that all checkouts will end up as staged revisions (one step only).

Philip

#20 lore raw
In reply to Philip Oakley
Quoted 1 line · Philip Oakley

I've added back the list, as it was accidentally dropped.

Thanks. I'm sorry. I apparently pressed the wrong button in my emails client. What I wrote is visible in your quote.

Quoted 16 lines

From: "Christoph Michelbach" <[email protected]>

Quoted 15 lines

I understand what the command does. It behaves perfectly as I expected
it to. I did not find this script but wrote it to demonstrate that what
the documentation says is different from how it behaves after having read what the documentation says it should do and noticing that that's
not how I expected it to work from experience.

What it really does is to copy all files described by the given paths
from the given tree-ish to the working directory. Or at least that's my
expectation of what it does.

Quoted 7 lines
Quoted 3 lines

The documentation, however, says that the given paths are *restored*.
This is different.

I don't see that difference in the phrase *restored*, compared to your 'copy
all files described by the given paths'. Could you explain a little more?

Suppose you have X. If you say X is restored to what it was half an hour ago, I expect everything about X to be exactly as it was half an hour ago. So if X is a file containing the text "Hello, world!", I expect there to be a file (with the exact same file name) which contains the text "Hello, world!", even if I changed that text in the mean time or deleted the file entirely. If X is a folder which contains files, I expect it to have the exact same contents as before. If there were 5 files in the folder half an hour ago, I expect there to be 5 files with the same file names and the same content in each file, again, even if I deleted, renamed, changed the contents of, or added files in the mean time. This is, however, not the case. Suppose I renamed a file from "foo" to "bar". Then there are 6 files, not 5. This is not how X was half an hour ago.

If you, however, say that all files in the path X are copied back from what they were half an hour ago, if X is a file, I expect the exact same thing as before for it. And that's actually what happens in reality. But if X is a folder, there's a difference: Because only the *files X contains* are copied back – *not X itself* –, all the files in X which do not occur in the state from half an hour ago, are unaffected. So now I expect there to be a 6th file because there is no file "bar" in the state from half an hour ago which could overwrite the file "bar" in the working tree.

Quoted 15 lines
Quoted 15 lines

Yeah, definitely. There should be 2 separate entries for this.

I think someone thought it was a good idea to make <pathspec>... optional in the synopsis because git checkout behaves in that special
way if a patch *or* paths are given. But then, of course, with both `-
p|--patch and <pathspec>...` optional, the command is the same as the
first variation, just with optional parameters -- but the documentation
(correctly) says those variations should behave very differently from
each other.

Quoted 4 lines
Quoted 3 lines

I don't see how this can be avoided without having 2 separate entries
for those cases.

true.

So now we have to find someone who used that command with both a patch and a tree-ish present a lot because I have no idea how that would behave and don't even know why you would use it.

--
Christoph

#21 lore raw
In reply to Philip Oakley
Quoted 5 lines · Philip Oakley

I'd guess that the misunderstanding is that you maybe thought that the whole directory would be reset to it's old state and the files b and c deleted, rather than just the named files present in that old commit being extracted. If we'd created and added a file d just before the checkout, what should have happened to d, and why?

It probably is a bit unfair to call it "misunderstanding". I've had this entry in the "Leftover Bits" list for quite some time:

git checkout $commit -- somedir may want to remove somedir/file that is not in $commit but is in the original index. Anybody who wants to do this needs to consider ramifications and devise transition plans. Cf. $gmane/234935

In the thread, this message:

https://public-inbox.org/git/[email protected]/

may be a good summary.

#22 lore raw
Quoted 21 lines · Junio C Hamano
Quoted 8 lines · Philip Oakley

I'd guess that the misunderstanding is that you maybe thought that the
whole directory would be reset to it's old state and the files b and c
deleted, rather than just the named files present in that old commit being extracted. If we'd created and added a file d just before the checkout, what should have happened to d, and why?

It probably is a bit unfair to call it "misunderstanding".  I've had this entry in the "Leftover Bits" list for quite some time:

    git checkout $commit -- somedir may want to remove somedir/file that
    is not in $commit but is in the original index. Anybody who wants to
    do this needs to consider ramifications and devise transition plans.
    Cf. $gmane/234935

Quoted 6 lines

In the thread, this message:

https://public-inbox.org/git/[email protected].
com/

may be a good summary.

For clarification: I'm not saying that that's how the command should behave. I merely want the documentation to describe how the command actually behaves.

Of course, being able to reset a folder to a previous state would be great, too. I have needed that several times and what I ended up doing is to delete the folder and then do the checkout. This doesn't cause problems if there are no untracked files (as of the state one wants to restore; what's currently tracked is of course irrelevant) you still need in that folder. If that's the case, one must make sure to not delete those files.

I don't know whether there's an easier/better way, though. That's merely what I did.

Having the *option* to reset an entire folder rather than the files in it would be great.

--
Christoph

#23 lore raw
In reply to Junio C Hamano

Thanks,

The link does contatin the ref to the original commit, which is helpful. (expands to https://github.com/git/git/commit/0a1283bc3955a97557)

Philip
[I'll be off-line all day]
----- Original Message ----- From: "Junio C Hamano" <[email protected]>
To: "Philip Oakley" <[email protected]>
Cc: "Christoph Michelbach" <[email protected]>; "Git Mailing List" <[email protected]>
Sent: Tuesday, April 18, 2017 1:26 AM
Subject: Re: [PATCH] Documentation/git-checkout: make doc. of checkout <tree-ish> clearer

Quoted 22 lines
Quoted 5 lines · Philip Oakley

I'd guess that the misunderstanding is that you maybe thought that the whole directory would be reset to it's old state and the files b and c deleted, rather than just the named files present in that old commit being extracted. If we'd created and added a file d just before the checkout, what should have happened to d, and why?

It probably is a bit unfair to call it "misunderstanding". I've had this entry in the "Leftover Bits" list for quite some time:

git checkout $commit -- somedir may want to remove somedir/file that is not in $commit but is in the original index. Anybody who wants to do this needs to consider ramifications and devise transition plans. Cf. $gmane/234935

In the thread, this message:

https://public-inbox.org/git/[email protected]/

may be a good summary.