I've added back the list, as it was accidentally dropped.
From: "Christoph Michelbach" <michelbach94@xxxxxxxxx>
On Mon, 2017-04-17 at 17:04 +0100, Philip Oakley wrote:
From: "Christoph Michelbach" <michelbach94@xxxxxxxxx>
>
> On Sun, 2017-04-16 at 22:25 +0100, Philip Oakley wrote:
> >
> > From: "Christoph Michelbach" <michelbach94@xxxxxxxxx>
> > >
> > > 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.
>
>
> >
> > 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).
>
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?
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.
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?
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.
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.
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.
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.
I don't see how this can be avoided without having 2 separate entries
for those cases.
true.
--
Philip