Suggestion: make git checkout safer

Suggestion: make git checkout safer

[Ed Avis]

Currently a plain 'git checkout .' will revert any local changes, e.g.

    % mkdir test
    % cd test
    % git init
    Initialized empty Git repository in /home/eda/test/.git/
    % echo hello >foo
    % git add foo
    % git commit -m.
    [master (root-commit) 34f6694] .
     1 file changed, 1 insertion(+)
     create mode 100644 foo
    % echo goodbye >foo
    % git checkout .
    % cat foo
    hello

I suggest this is dangerous and by default 'git checkout' should only alter
files which do not have local changes (as would be reported by 'git diff').
Only if --force is given should working tree differences be thrown away.

    % git --version
    git version 2.4.0

-- 
Ed Avis <eda@waniasset.com>

Re: Suggestion: make git checkout safer

[Jeff King]

On Wed, Jun 03, 2015 at 08:50:44AM +0000, Ed Avis wrote:

> Currently a plain 'git checkout .' will revert any local changes, e.g.
> 
>     % mkdir test
>     % cd test
>     % git init
>     Initialized empty Git repository in /home/eda/test/.git/
>     % echo hello >foo
>     % git add foo
>     % git commit -m.
>     [master (root-commit) 34f6694] .
>      1 file changed, 1 insertion(+)
>      create mode 100644 foo
>     % echo goodbye >foo
>     % git checkout .
>     % cat foo
>     hello
> 
> I suggest this is dangerous and by default 'git checkout' should only alter
> files which do not have local changes (as would be reported by 'git diff').
> Only if --force is given should working tree differences be thrown away.
> 
>     % git --version
>     git version 2.4.0

That's what "git checkout <path>" is designed for. I'm not clear on what
you expect "git checkout ." to do in this example, if not overwrite
"foo". Can you elaborate?

-Peff

Re: Suggestion: make git checkout safer

[Ed Avis]

I had expected that 'git checkout .' would fix up my working tree to make it
match the repository (in this case, the current revision of the master
branch).  When I originally ran it I had deleted a couple of files from the
working tree and wanted to restore them.  However, I expected that if doing
the checkout operation would lose data currently on disk then git would
prompt me first.

To compare, 'git pull' will not silently overwrite local changes; it will
prompt you to commit or stash them first.  'git checkout .' is a fairly
innocuous-looking command; it doesn't contain any --force or --overwrite or
other things that would make you think twice before typing it.  So I suggest
it should be equally safe to run.

The user interface might be something like:

% git checkout .
error: Your local changes to the following files would be overwritten:
        foo
You may want to commit or stash these changes, or delete the files if you
don't want them.  Use 'git checkout --force' to proceed, throwing away
local changes.
Aborting

If the checkout operation would only involve creating some files on disk
which aren't currently there, then it would proceed without prompting.

-- 
Ed Avis <eda@waniasset.com>

Re: Suggestion: make git checkout safer

[Jeff King]

On Wed, Jun 03, 2015 at 09:21:59AM +0000, Ed Avis wrote:

> I had expected that 'git checkout .' would fix up my working tree to make it
> match the repository (in this case, the current revision of the master
> branch).

It did. :)

> The user interface might be something like:
> 
> % git checkout .
> error: Your local changes to the following files would be overwritten:
>         foo
> You may want to commit or stash these changes, or delete the files if you
> don't want them.  Use 'git checkout --force' to proceed, throwing away
> local changes.
> Aborting
> 
> If the checkout operation would only involve creating some files on disk
> which aren't currently there, then it would proceed without prompting.

Thanks for explaining. I see where you are coming from, though I'm still
a bit lukewarm on the idea, if only because the vast majority of
invocations would involve "--force".

It also seems a bit special-cased to treat restoring deletions
specially.  I would say the more "usual" way to use checkout like this
is to give specific paths. I.e., run "git status", say "oh, I need to
restore the contents of 'foo', but not 'bar'", and run "git checkout
foo". That works regardless of the type of change to "foo" and "bar".

If we want to introduce more safety here, I'd be inclined to perform the
operation by default, but give a better escape hatch. For example, by
creating a loose object for any file we're about to overwrite, and
possibly writing an entry into a log. That's a lot more work, but has a
few advantages:

  1. It helps even when you just ran with "--force" followed by an
     "oops, why did I do that?" moment.

  2. It can help other commands like "git clean".

  3. That log could form a basis for a "git undo" program to help with
     "oops" moments in general (e.g., if you use "git reset ." to
     overwrite what is in the index, we have all of the old file content
     in objects, but it can sometimes be a pain to figure out _which_
     objects went where.

-Peff

Re: Suggestion: make git checkout safer

[Ed Avis]

Jeff King <peff <at> peff.net> writes:

>I would say the more "usual" way to use checkout like this
>is to give specific paths. I.e., run "git status", say "oh, I need to
>restore the contents of 'foo', but not 'bar'", and run "git checkout
>foo". That works regardless of the type of change to "foo" and "bar".

That seems fine - a specific file is named and you clearly want to alter
the contents of that file.  By analogy, 'rm foo' will silently delete it,
but if you specify a directory to delete recursively you need the -r flag.
OK, it's not a perfect analogy because the purpose of rm is to delete data
and nothing else ;-).

If my personal experience is anything to go by, newcomers may fall into the
habit of running 'git checkout .' to restore missing files.  In the old days
I would often delete a file and then run 'cvs update' or 'svn update' to
restore it.  That would fetch a fresh copy from the repository, and while
it might do some kind of diff/patch operation on modified files, it would
not simply throw away local changes.

'git checkout .' seems like the analogous command, but it has much sharper
edges.  I still think it should be safer by default, but if you decide
against that then perhaps you need to create some way to restore missing
files and not overwrite others.  'git checkout --no-overwrite'?  Then it
could even be added to .gitconfig as the default for those who like it.

I have to say that as a newcomer to git I do not like the idea of creating
a special undo log for git.  It would just be yet another concept to learn
and another thing to add to the list of 'where is git hiding my data this
time?'.  And the time when it would be useful - after some bungled operation
that lost data - is just the time when the user is already confused and
adding another semi-hidden stash of objects to the mix would befuddle them
further.  If there is to be a backup made of local changes that get lost,
and I agree it is a good idea, then it should be something stupid and
completely obvious, such as saving the old file as 'foo.before_checkout.1'.

-- 
Ed Avis <eda@waniasset.com>

Re: Suggestion: make git checkout safer

[Junio C Hamano]

Ed Avis <eda@waniasset.com> writes:

> If my personal experience is anything to go by, newcomers may fall into the
> habit of running 'git checkout .' to restore missing files.

Is that really true?  It all depends on why you came to a situation
to have "missing files" in the first place, I would think, but "git
checkout $path" is "I messed up the version in the working tree at
$path, and want to restore them".  One particular kind of "I messed
up" may be "I deleted by mistake" (hence making them "missing"), but
is it so common to delete things by mistake, as opposed to editing,
making a mess and realizing that the work so far was not improving
things and wanting to restart from scratch?

RE: Suggestion: make git checkout safer

[Randall S. Becker]

On June 3, 2015 1:35 PM Junio C Hamano wrote:
> Ed Avis <eda@waniasset.com> writes:
> > If my personal experience is anything to go by, newcomers may fall
> > into the habit of running 'git checkout .' to restore missing files.
> Is that really true?  It all depends on why you came to a situation to
have
> "missing files" in the first place, I would think, but "git checkout
$path" is "I
> messed up the version in the working tree at $path, and want to restore
them".
> One particular kind of "I messed up" may be "I deleted by mistake" (hence
> making them "missing"), but is it so common to delete things by mistake,
as
> opposed to editing, making a mess and realizing that the work so far was
not
> improving things and wanting to restart from scratch?

When working in an IDE like ECLIPSE or MonoDevelop, accidentally hitting the
DEL button or a drag-drop move is a fairly common trigger for the
"Wait-No-Stop-Oh-Drats" process which includes running git checkout to
recover. My keyboard is excessively sensitive static, so this happens more
often than I will admit (shamelessly blaming hardware when it really is a
user problem). Git checkout is a life-saver in this case as is frequently
committing. :)

Cheers,
Randall

-- Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)/NonStop(211288444200000000)
-- In my real life, I talk too much.

Re: Suggestion: make git checkout safer

[Junio C Hamano]

"Randall S. Becker" <rsbecker@nexbridge.com> writes:

> On June 3, 2015 1:35 PM Junio C Hamano wrote:
>> Is that really true?  It all depends on why you came to a
>> situation to have "missing files" in the first place, I would
>> think, but "git checkout $path" is "I messed up the version in
>> the working tree at $path, and want to restore them".  One
>> particular kind of "I messed up" may be "I deleted by mistake"
>> (hence making them "missing"), but is it so common to delete
>> things by mistake, as opposed to editing, making a mess and
>> realizing that the work so far was not improving things and
>> wanting to restart from scratch?
>
> When working in an IDE like ECLIPSE or MonoDevelop, accidentally
> hitting the DEL button or a drag-drop move is a fairly common
> trigger for the "Wait-No-Stop-Oh-Drats" process which includes
> running git checkout to recover.

That is an interesting tangent.  If you are lucky then the deleted
file may be unedited one, but I presume that you are not always
lucky.  So perhaps "git checkout" is not a solution to that
particular IDE issue in the first place?

RE: Suggestion: make git checkout safer

[Randall S. Becker]

On June 3, 2015 2:11 PM Junio C Hamano wrote:
> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> > On June 3, 2015 1:35 PM Junio C Hamano wrote:
> >> Is that really true?  It all depends on why you came to a situation
> >> to have "missing files" in the first place, I would think, but "git
> >> checkout $path" is "I messed up the version in the working tree at
> >> $path, and want to restore them".  One particular kind of "I messed
> >> up" may be "I deleted by mistake"
> >> (hence making them "missing"), but is it so common to delete things
> >> by mistake, as opposed to editing, making a mess and realizing that
> >> the work so far was not improving things and wanting to restart from
> >> scratch?
> >
> > When working in an IDE like ECLIPSE or MonoDevelop, accidentally
> > hitting the DEL button or a drag-drop move is a fairly common trigger
> > for the "Wait-No-Stop-Oh-Drats" process which includes running git
> > checkout to recover.
> 
> That is an interesting tangent.  If you are lucky then the deleted file
may be
> unedited one, but I presume that you are not always lucky.  So perhaps
"git
> checkout" is not a solution to that particular IDE issue in the first
place?

Agreed. That's why I like knowing what's in my sausages and commit often.
Only lost a minor change once from this. I wonder what else is afoot. Ed,
can you expand on the issue?

Re: Suggestion: make git checkout safer

[Stefan Beller]

Maybe the expectation comes from the existing warnings when checking
out branches?

    $ mkdir tmp && cd tmp
    $ git init
    $ echo Hello >foo
    $ git add foo
    $ git commit -am "Hello"
    $ git branch next
    $ echo "world" >bar
    $ git add bar
    $ git commit -a -m "World"
    $ git checkout test
      # no problem so far, just going back one commit on anther branch
    $ echo Kitty >bar
    $ git checkout master # now we get it:
    error: The following untracked working tree files would be
overwritten by checkout:
    bar
    Please move or remove them before you can switch branches.
    Aborting

So in one mode, we do actually warn about contents going missing, and the other
mode is designed to actually make things go missing without any warning.

So maybe the checkout command is *too powerful* ? Looking at the man page:

    Updates files in the working tree to match the version in the index or the
    specified tree. If no paths are given, git checkout will also update HEAD
    to set the specified branch as the current branch.

we're mixing two different tasks here anyway. "Updating files in the work tree"
can be understood as "throwing away all changes until you're back at a specified
safe point". If I were to come up with a name for such an action it's
maybe "reset" or
"reset-file(s)". Though git reset is taken already and does different things.
"reset" sounds as if stuff may go missing, so anyone who types
"reset", (even without
exactly understanding what it does, would assume it is as safe as
typing "rm" probably.

 And "also update HEAD" can be understood as "switch to another branch",
so if I were to invent a new porcelain command for such functionality it may be
called "git switch-branch". And typing switch-branch would be expected
to carry all
the warnings (no updating files in the work tree, when in danger of
losing its content)

Re: Suggestion: make git checkout safer

[Ed Avis]

Stefan Beller <sbeller <at> google.com> writes:

>So in one mode, we do actually warn about contents going missing, and the
>other mode is designed to actually make things go missing without any
>warning.

I think this is a big part of the issue.  Two rather different operations
are given the name 'checkout', and the safety standards applied to them
also differ greatly.  The manual page doesn't make it clear that it can
be quite a dangerous command to run, even without --force.

>If I were to come up with a name for such an action it's
>maybe "reset" or "reset-file(s)".

Agreed.  Or 'git clean' could become more powerful and able to reset file
contents as well as deleting untracked files.  The name and documentation of
'git clean' already make it clear that it's not something safe to run without
thinking first.

Julio H. asked how I had learned to run 'git checkout .'.  I think it was just
word of mouth.  I had deleted some files from the working tree and asked a
colleague how to restore them from the repository - which is, after all, a
bread-and-butter operation for any version control system.  What is the
correct command to run, then, to safely restore missing files?

And yes, it probably would be better to use git's native mechanisms to throw
away local changes to a file, rather than the sledgehammer approach of just
deleting it and checking it out again.  Most of the time I do so.  Sometimes
when everything is a real mess it is more straighforward to reach for 'rm' -
or indeed for the delete option in your IDE or file browser.

-- 
Ed Avis <eda@waniasset.com>

Re: Suggestion: make git checkout safer

[Ed Avis]

Ed Avis <eda <at> waniasset.com> writes:

>Julio H. asked how I had learned to run 'git checkout .'.

Sorry it was Torsten B. who asked that.  But yes, I think it was just word
of mouth.

-- 
Ed Avis <eda@waniasset.com>

Re: Suggestion: make git checkout safer

[Torsten Bögershausen]

On 2015-06-03 11.55, Ed Avis wrote:
> Jeff King <peff <at> peff.net> writes:
> 
>> I would say the more "usual" way to use checkout like this
>> is to give specific paths. I.e., run "git status", say "oh, I need to
>> restore the contents of 'foo', but not 'bar'", and run "git checkout
>> foo". That works regardless of the type of change to "foo" and "bar".
> 
> That seems fine - a specific file is named and you clearly want to alter
> the contents of that file.  By analogy, 'rm foo' will silently delete it,
> but if you specify a directory to delete recursively you need the -r flag.
> OK, it's not a perfect analogy because the purpose of rm is to delete data
> and nothing else ;-).
> 
> If my personal experience is anything to go by, newcomers may fall into the
> habit of running 'git checkout .' to restore missing files.  In the old days
> I would often delete a file and then run 'cvs update' or 'svn update' to
> restore it.  That would fetch a fresh copy from the repository, and while
> it might do some kind of diff/patch operation on modified files, it would
> not simply throw away local changes.
> 
> 'git checkout .' seems like the analogous command, but it has much sharper
> edges.  I still think it should be safer by default, but if you decide
> against that then perhaps you need to create some way to restore missing
> files and not overwrite others.  'git checkout --no-overwrite'?  Then it
> could even be added to .gitconfig as the default for those who like it.
> 
> I have to say that as a newcomer to git I do not like the idea of creating
> a special undo log for git.  It would just be yet another concept to learn
> and another thing to add to the list of 'where is git hiding my data this
> time?'.  And the time when it would be useful - after some bungled operation
> that lost data - is just the time when the user is already confused and
> adding another semi-hidden stash of objects to the mix would befuddle them
> further.  If there is to be a backup made of local changes that get lost,
> and I agree it is a good idea, then it should be something stupid and
> completely obvious, such as saving the old file as 'foo.before_checkout.1'.
> 
This is what my Git says:

git status
On branch master
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   A
        deleted:    B

(So it should be somewhat self-documenting)


I try to avoid things like "git reset --hard", and "git checkout .",
and often use "git stash" instead.

It may be that there is a chance to improve the documentation.

Just for curiosity:
From where did you got the information to run "git checkout ." ?

Re: Suggestion: make git checkout safer

[Kevin Daudt]

On Wed, Jun 03, 2015 at 09:55:05AM +0000, Ed Avis wrote:
> Jeff King <peff <at> peff.net> writes:
> 
 
> If my personal experience is anything to go by, newcomers may fall into the
> habit of running 'git checkout .' to restore missing files.  In the old days
> I would often delete a file and then run 'cvs update' or 'svn update' to
> restore it.  That would fetch a fresh copy from the repository, and while
> it might do some kind of diff/patch operation on modified files, it would
> not simply throw away local changes.
> 

The problem with these kinds of habbits is that they easily extend to
the --force variant. If people execute git checkout . as a habbit
without thinking, they will soon train to do git checkout -f . without
thinking, and then you still have the same problem.

I do share your sentiment that it's easy to loose uncomitted changes to
git checkout <path>, but like Jeff said, the entire goal of this command
is to reset specific files from the index or commits. 

Introducing a way to undo this would be a much better option to me then
adding an extra switch with no way to undo.

Re: Suggestion: make git checkout safer

[Ed Avis]

Kevin Daudt <me <at> ikke.info> writes:

>If people execute git checkout . as a habbit
>without thinking, they will soon train to do git checkout -f . without
>thinking, and then you still have the same problem.

I don't quite agree; you only learn to use the -f flag if the plain command
doesn't do what you want.  With rm, I want to remove that file, dammit!  The
-f flag is often a necessity to stop the tool getting in my way.  But when
fixing up a working tree, I rarely want to silently trash any local changes.

>I do share your sentiment that it's easy to loose uncomitted changes to
>git checkout <path>, but like Jeff said, the entire goal of this command
>is to reset specific files from the index or commits.

Well that's not quite the flavour given by the documentation, which says

    >Updates files in the working tree to match...

'Updating' files sounds like a fairly safe thing to do, right?  Like 'cvs
update' or 'svn update', which don't just overwrite working tree changes.
The doc doesn't really make clear that any local changes will be discarded;
indeed the only mention of that is

       -f, --force
         When switching branches... this is used to throw away local changes.

To the casual reader, following 'the exception proves the rule', it appears
that local changes are not thrown away except in this case.

-- 
Ed Avis <eda@waniasset.com>

Re: Suggestion: make git checkout safer

[Torsten Bögershausen]

On 2015-06-04 13.00, Ed Avis wrote:
> 
>     >Updates files in the working tree to match...
I think that this had been written with
"git checkout <branch>" in mind, which is different
from "git checkout -- <paths>" (or "git checkout .")

Do you think you can write a patch to improve the documentation ?

Re: Suggestion: make git checkout safer

[Ed Avis]

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

>Do you think you can write a patch to improve the documentation ?

Here is my attempt, but it is only a starting point.

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index d263a56..ee25354 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 - Overwrite working tree files with a given branch

 SYNOPSIS
 --------
@@ -17,10 +17,11 @@ SYNOPSIS

 DESCRIPTION
 -----------
-Updates files in the working tree to match the version in the index
-or the specified tree.  If no paths are given, 'git checkout' will
-also update `HEAD` to set the specified branch as the current
-branch.
+Updates, creates, or overwrites files in the working tree to match the
+version in the index or the specified tree.  If no paths are given,
+'git checkout' will also update `HEAD` to set the specified branch as
+the current branch, and will keep local changes.  If paths are given,
+'git checkout' will unconditionally overwrite local changes.

 'git checkout' <branch>::
        To prepare for working on <branch>, switch to it by updating
@@ -81,21 +82,24 @@ 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.
-+
-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.
-Using `-f` will ignore these unmerged entries.  The contents from a
-specific side of the merge can be checked out of the index by
-using `--ours` or `--theirs`.  With `-m`, changes made to the working tree
-file can be discarded to re-create the original conflicted merge result.
+       switch branches.  It overwrites the named paths in the working
+       tree from the index file or from a named <tree-ish> (most
+       often a commit).  Unlike other modes, local modifications to
+       the files in the working tree are *not* kept.
+
+        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.  + 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.  Using `-f` will ignore these unmerged entries.  The
+       contents from a specific side of the merge can be checked out
+       of the index by using `--ours` or `--theirs`.  With `-m`,
+       changes made to the working tree file can be discarded to
+       re-create the original conflicted merge result.

 OPTIONS
 -------
@@ -110,7 +114,9 @@ OPTIONS
        local changes.
 +
 When checking out paths from the index, do not fail upon unmerged
-entries; instead, unmerged entries are ignored.
+entries; instead, unmerged entries are ignored.  (Note that when
+checking out paths, local changes are thrown away whether or not
+this flag is given.)

 --ours::
 --theirs::
@@ -481,10 +487,10 @@ $ git checkout hello.c            <3>
 ------------
 +
 <1> switch branch
-<2> take a file out of another commit
-<3> restore hello.c from the index
+<2> take a file out of another commit, overwriting any local changes
+<3> restore hello.c from the index (would overwrite it if it existed)
 +
-If you want to check out _all_ C source files out of the index,
+If you want to revert _all_ C source files out of the index,
 you can say
 +
 ------------
@@ -492,7 +498,7 @@ $ git checkout -- '*.c'
 ------------
 +
 Note the quotes around `*.c`.  The file `hello.c` will also be
-checked out, even though it is no longer in the working tree,
+created, even though it is no longer in the working tree,
 because the file globbing is used to match entries in the index
 (not in the working tree by the shell).

Re: Suggestion: make git checkout safer

[Duy Nguyen]

On Fri, Jun 5, 2015 at 4:32 PM, Ed Avis <eda@waniasset.com> wrote:
> Torsten Bögershausen <tboegi <at> web.de> writes:
>
>>Do you think you can write a patch to improve the documentation ?
>
> Here is my attempt, but it is only a starting point.
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index d263a56..ee25354 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 - Overwrite working tree files with a given branch

Maybe "switch branches or reset working tree files"?
-- 
Duy

Re: Suggestion: make git checkout safer

[Eric Sunshine]

On Fri, Jun 5, 2015 at 5:32 AM, Ed Avis <eda@waniasset.com> wrote:
> Torsten Bögershausen <tboegi <at> web.de> writes:
>>Do you think you can write a patch to improve the documentation ?
>
> Here is my attempt, but it is only a starting point.
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index d263a56..ee25354 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 - Overwrite working tree files with a given branch

I agree with Duy's suggestion of "switch branches or reset working
tree files" since it explains the high-level purpose of the command,
whereas your wording gives details of the low-level operation without
conveying the high-level purpose.

>  SYNOPSIS
>  --------
> @@ -17,10 +17,11 @@ SYNOPSIS
>
>  DESCRIPTION
>  -----------
> -Updates files in the working tree to match the version in the index
> -or the specified tree.  If no paths are given, 'git checkout' will
> -also update `HEAD` to set the specified branch as the current
> -branch.
> +Updates, creates, or overwrites files in the working tree to match the
> +version in the index or the specified tree.  If no paths are given,
> +'git checkout' will also update `HEAD` to set the specified branch as
> +the current branch, and will keep local changes.

The two changes you made here don't really work together, do they? In
the one case, you changed "Update" to "Update, creates, or
overwrites", and in the second you added "and will keep local changes"
which seems to contradict the "overwrites" bit.

Moreover, as this paragraph is a high-level overview of the modes
enumerated below, it doesn't necessarily make sense to go into such
detail here. For instance, the "git checkout <branch>" case
immediately below this paragraph already says clearly "Local
modifications to the files in the working tree are kept", so repeating
it here seems unnecessary.

> +If paths are given,
> +'git checkout' will unconditionally overwrite local changes.

Likewise, I'm not convinced that it makes sense to add this sentence.
The preceding sentence about updating HEAD applies to all of the
enumerated cases except the "git checkout <pathspec>…" case, so it
makes sense to have it in the overview paragraph, however, this new
sentence applies only to the one case, so its placement here is
questionable.

>  'git checkout' <branch>::
>         To prepare for working on <branch>, switch to it by updating
> @@ -81,21 +82,24 @@ 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.
> -+
> -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.
> -Using `-f` will ignore these unmerged entries.  The contents from a
> -specific side of the merge can be checked out of the index by
> -using `--ours` or `--theirs`.  With `-m`, changes made to the working tree
> -file can be discarded to re-create the original conflicted merge result.
> +       switch branches.  It overwrites the named paths in the working
> +       tree from the index file or from a named <tree-ish> (most
> +       often a commit).

Rather than "updates" or "overwrites", how about something like this?

    "It restores the named paths in the working tree
    to a pristine state from the index..."

> +Unlike other modes, local modifications to
> +       the files in the working tree are *not* kept.

This sentence seems utterly redundant with the sentence immediately
preceding it, thus adds noise but no obvious value.

> +        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.  + 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.  Using `-f` will ignore these unmerged entries.  The
> +       contents from a specific side of the merge can be checked out
> +       of the index by using `--ours` or `--theirs`.  With `-m`,
> +       changes made to the working tree file can be discarded to
> +       re-create the original conflicted merge result.

A couple issues:

In order for this to format correctly in Asciidoc, it must be
unindented (as in the original), and you must keep the unindented "+"
on the line by itself before the paragraph to tie it to the preceding
paragraph.

You mis-wrapped and combined two formerly separate paragraphs into
one. Notice the "+ The" in the middle of your re-wrapped paragraph.

>  OPTIONS
>  -------
> @@ -110,7 +114,9 @@ OPTIONS
>         local changes.
>  +
>  When checking out paths from the index, do not fail upon unmerged
> -entries; instead, unmerged entries are ignored.
> +entries; instead, unmerged entries are ignored.  (Note that when
> +checking out paths, local changes are thrown away whether or not
> +this flag is given.)

I don't see what value this change adds. The description of "git
checkout <pathspec>…" already explains that named paths are restored
to a pristine state, so talking about it here may give the false
impression that the reader must be aware that something unusual or
unexpected is going on beyond what was already documented.

>  --ours::
>  --theirs::
> @@ -481,10 +487,10 @@ $ git checkout hello.c            <3>
>  ------------
>  +
>  <1> switch branch
> -<2> take a file out of another commit
> -<3> restore hello.c from the index
> +<2> take a file out of another commit, overwriting any local changes
> +<3> restore hello.c from the index (would overwrite it if it existed)

Again, I don't see what value these changes add. These
example-specific notes are intended to explain what is happening in
the example. These new clauses don't do that, thus making them
potentially confusing.

To really convey that local changes will be overwritten, how about
adding a new example which demonstrates it explicitly?

> -If you want to check out _all_ C source files out of the index,
> +If you want to revert _all_ C source files out of the index,
>  you can say

How about?

    If you want to restore _all_ C source files to a
    pristine state from the index, you can say

>  +
>  ------------
> @@ -492,7 +498,7 @@ $ git checkout -- '*.c'
>  ------------
>  +
>  Note the quotes around `*.c`.  The file `hello.c` will also be
> -checked out, even though it is no longer in the working tree,
> +created, even though it is no longer in the working tree,

Again:

    ...`hello.c` will also be restored,...

>  because the file globbing is used to match entries in the index
>  (not in the working tree by the shell).

Re: Suggestion: make git checkout safer

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> ...
> Again:
>
>     ...`hello.c` will also be restored,...
>
>>  because the file globbing is used to match entries in the index
>>  (not in the working tree by the shell).

Thanks for a thorough review.  I agree with all the comments and
suggestions you gave.  Also, Ed, thanks for an attempt to improve
the documentation.

I think the biggest problem with this patch is that the tone of the
updated text is geared a lot more towards venting the initial
frustration of the writer than helping the readers of the document.

By explaining what the behaviour is meant to solve and help, the
readers would get useful information (e.g. "this is to be used to
restore pristine contents").  The same thing said in the negative
way only serve to unnecessarily repel readers (e.g. "this will
unconditionally overwrite and lose contents").

Technically, they are the descriptions of the same thing---in order
to restore pristine contents to the workng tree, you have to discard
the botched changes you made in the working tree, and that is done
"unconditionally" by "overwriting" and "losing contents".  But
saying it in the negative way does not serve as a useful warning.

The readers are intelligent, and they will understand (and will even
appreciate) that a request to replace their botched contents in the
working tree out of the index is done unconditionally without being
asked an unnecessary "are you sure?" and done by overwriting the
files, losing the botched contents from there, once they are
explained why they want to "git checkout $paths", what the operation
is meant to be used for.

Perhaps taking a deep breath and waiting for a few days for the head
to coll down and frustrations to dissipate may be a good thing to do
;-)

Re: Suggestion: make git checkout safer

[Ed Avis]

I'm not attached to the wording changes posted earlier.  As I said, it is
only a starting point.

I do feel that 'git checkout PATH' is rather a dangerous operation, and
moreover a surprisingly dangerous one, since 'git checkout BRANCH' is
careful not to lose local changes, as are other common commands like
'git pull'.  In the documentation patch I tried to highlight the
distinction between the two rather different, and perhaps even
Jekyll-and-Hyde-like, modes of this command.

But rather than adding heavyhanded and redundant warnings to the
documentation it would be better for the command not to be quite so
sharp-edged.  There is already a --force option for one mode, which could
easily be made to apply to the other too (so local changes will not be
discarded unless --force is given).  Is the only argument against it that
'git checkout is intended to overwrite changes'?  That seems a little
circular since the question is whether its intended behaviour could change
to something a little safer.  Surely a sensible Huffman-coding of git
commands would give longer and harder-to-type names like 'git checkout
--force .' to relatively dangerous operations?

Or indeed, split out the two different modes into two separate commands.
The job of reverting file contents seems like something for 'git clean'.

I've said all I have to say but I would like to ask, in the hope of becoming
a better git user: if 'git checkout .' is not a safe way to restore missing
files in the working tree, what is the recommended way to do that?

Thanks all for your comments and guidance.

-- 
Ed Avis <eda@waniasset.com>

Re: Suggestion: make git checkout safer

[Eric Sunshine]

On Fri, Jun 5, 2015 at 1:44 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Fri, Jun 5, 2015 at 5:32 AM, Ed Avis <eda@waniasset.com> wrote:
>> Torsten Bögershausen <tboegi <at> web.de> writes:
>>>Do you think you can write a patch to improve the documentation ?
>>
>> Here is my attempt, but it is only a starting point.
>>
>> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
>> index d263a56..ee25354 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 - Overwrite working tree files with a given branch
>
> I agree with Duy's suggestion of "switch branches or reset working

I meant, but forgot to say, that I'd probably replace "reset" with
"restore" in Duy's suggestion.

> tree files" since it explains the high-level purpose of the command,
> whereas your wording gives details of the low-level operation without
> conveying the high-level purpose.

Re: Suggestion: make git checkout safer

[Philip Oakley]

From: "Ed Avis" <eda@waniasset.com>
Sent: Wednesday, June 03, 2015 10:55 AM
> Jeff King <peff <at> peff.net> writes:
>
>>I would say the more "usual" way to use checkout like this
>>is to give specific paths. I.e., run "git status", say "oh, I need to
>>restore the contents of 'foo', but not 'bar'", and run "git checkout
>>foo". That works regardless of the type of change to "foo" and "bar".
>
> That seems fine - a specific file is named and you clearly want to 
> alter
> the contents of that file.  By analogy, 'rm foo' will silently delete 
> it,
> but if you specify a directory to delete recursively you need the -r 
> flag.
> OK, it's not a perfect analogy because the purpose of rm is to delete 
> data
> and nothing else ;-).
>
> If my personal experience is anything to go by, newcomers may fall 
> into the
> habit of running 'git checkout .' to restore missing files.  In the 
> old days
> I would often delete a file and then run 'cvs update' or 'svn update' 
> to
> restore it.  That would fetch a fresh copy from the repository, and 
> while
> it might do some kind of diff/patch operation on modified files, it 
> would
> not simply throw away local changes.
>
> 'git checkout .' seems like the analogous command, but it has much 
> sharper
> edges.  I still think it should be safer by default, but if you decide
> against that then perhaps you need to create some way to restore 
> missing
> files and not overwrite others.  'git checkout --no-overwrite'?  Then 
> it
> could even be added to .gitconfig as the default for those who like 
> it.
>
> I have to say that as a newcomer to git I do not like the idea of 
> creating
> a special undo log for git.  It would just be yet another concept to 
> learn
> and another thing to add to the list of 'where is git hiding my data 
> this
> time?'.  And the time when it would be useful - after some bungled 
> operation
> that lost data - is just the time when the user is already confused 
> and
> adding another semi-hidden stash of objects to the mix would befuddle 
> them
> further.  If there is to be a backup made of local changes that get 
> lost,
> and I agree it is a good idea, then it should be something stupid and
> completely obvious, such as saving the old file as 
> 'foo.before_checkout.1'.
>
> -- 
> Ed Avis <eda@waniasset.com>
>

To me, when I saw the 'git checkout .', I was reminded of the 'git push 
. <refs>' special case where '.' is the repo, so in my mind the first 
thought was that Ed wanted to checkout the head of the current repo, and 
that should have barfed from that viewpoint.

The [is it equivalent? (rhet)] 'git checkout -- .' would clearly 
indicate that the '.' refers to the files of the current directory 
(wouldn't it?)

So it's about how '.' is perceived by the code in different 
circumstances, and whether, perhaps, the optional discriminating '--' 
should be required in this (special) case.

Philip 

Re: Suggestion: make git checkout safer

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> If we want to introduce more safety here, I'd be inclined to perform the
> operation by default, but give a better escape hatch. For example, by
> creating a loose object for any file we're about to overwrite, and
> possibly writing an entry into a log.

Can we borrow the ideas from other tools that have similar
characteristics, I wonder.

"git checkout $paths" (and you can give "." for $paths to mean
"everything") is akin to "cp -R $elsewhere/$path ." to restore the
working tree copies from somewhere else.

"Ouch, 'git checkout .'  overwrote what was in my working tree" is
exactly the same kind of confusion as "I ran 'cp -r ../saved .' and
it overwrote everything".  As you said in your initial response,
that is what the command is meant for.

What does that similar command outside world, "cp", have for "more
safety"?  'cp -i' asks if the user wants to overwrite a file for
each path; perhaps a behaviour similar to that was the original
poster wanted to see?

Re: Suggestion: make git checkout safer

[Jeff King]

On Wed, Jun 03, 2015 at 10:32:40AM -0700, Junio C Hamano wrote:

> "git checkout $paths" (and you can give "." for $paths to mean
> "everything") is akin to "cp -R $elsewhere/$path ." to restore the
> working tree copies from somewhere else.
> 
> "Ouch, 'git checkout .'  overwrote what was in my working tree" is
> exactly the same kind of confusion as "I ran 'cp -r ../saved .' and
> it overwrote everything".  As you said in your initial response,
> that is what the command is meant for.
> 
> What does that similar command outside world, "cp", have for "more
> safety"?  'cp -i' asks if the user wants to overwrite a file for
> each path; perhaps a behaviour similar to that was the original
> poster wanted to see?

Yeah, I'd say "cp -i" is the closest thing. I don't have a problem with
adding that, but I'd really hate for it to be the default (just as I
find distros which "alias rm='rm -i" annoying).

-Peff

RE: Suggestion: make git checkout safer

[Randall S. Becker]

On June 3, 2015 3:06 PM Jeff King wrote:
> On Wed, Jun 03, 2015 at 10:32:40AM -0700, Junio C Hamano wrote:
> > "git checkout $paths" (and you can give "." for $paths to mean
> > "everything") is akin to "cp -R $elsewhere/$path ." to restore the
> > working tree copies from somewhere else.
> >
> > "Ouch, 'git checkout .'  overwrote what was in my working tree" is
> > exactly the same kind of confusion as "I ran 'cp -r ../saved .' and it
> > overwrote everything".  As you said in your initial response, that is
> > what the command is meant for.
> >
> > What does that similar command outside world, "cp", have for "more
> > safety"?  'cp -i' asks if the user wants to overwrite a file for each
> > path; perhaps a behaviour similar to that was the original poster
> > wanted to see?
> 
> Yeah, I'd say "cp -i" is the closest thing. I don't have a problem with adding that,
> but I'd really hate for it to be the default (just as I find distros which "alias
> rm='rm -i" annoying).

Brainstorming a few compromises:

or some such config option to turn on behaviour like this:
core.checkout=-i

or some such thing where if there are strictly more than m files being touched and strictly less than n files to act accordingly - a threshold concept:
core.checkout_warn_upperlimit=n # default to 0
core.checkout_warn_lowerlimit=m # default to 0

or in a more gross fashion provide a pre-checkout hook to do all the work of prompting/control of the situation.

Personally I'm happy with the defaults as they are (and was not a fan of defaulting rm -i or cp -i either) but I can see the point and have had diffuse whines from my team on the checkout subject, which is why I'm commenting.

Re: Suggestion: make git checkout safer

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Jun 03, 2015 at 10:32:40AM -0700, Junio C Hamano wrote:
>
> Yeah, I'd say "cp -i" is the closest thing. I don't have a problem with
> adding that, but I'd really hate for it to be the default (just as I
> find distros which "alias rm='rm -i" annoying).

Oh, no question about it.

I think a typical user cease to be a newbie before having to type
"-i" every time starts to annoy her, and instead will learn to use
the tool more effectively and efficiently [*1*], so making "-i"
default is not good not just for you but for everybody.


[Footnote]

*1* In the context of this discussion, after screwing up the change
    in hello.c, instead of expressing the wish to recover and to
    start from scratch in two separate commands, i.e.

	rm hello.c && update-from-scm

    they will learn to use a single command that is designed for
    that purpose, i.e.

	checkout-from-scm hello.c

    without the "rm" step, which _is_ an artificial workaround for
    their other SCMs that do not update from the repository unless
    they remove the files.

Re: Suggestion: make git checkout safer

[John Szakmeister]

On Wed, Jun 3, 2015 at 5:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
[snip]
> [Footnote]
>
> *1* In the context of this discussion, after screwing up the change
>     in hello.c, instead of expressing the wish to recover and to
>     start from scratch in two separate commands, i.e.
>
>         rm hello.c && update-from-scm
>
>     they will learn to use a single command that is designed for
>     that purpose, i.e.
>
>         checkout-from-scm hello.c
>
>     without the "rm" step, which _is_ an artificial workaround for
>     their other SCMs that do not update from the repository unless
>     they remove the files.

Just to be clear, Subversion doesn't require you to remove the file to
restore it (I'm sure most of you know that, but just in case others
didn't).  There is a one-step way to restore the file:

    svn revert hello.c

Unfortunately, revert in the Git sense is about reverting commits, so
there's a bit of friction between Subversion and Git's terminology.
OTOH, once the team was educated how to think about it, "git checkout
<path>" has been pretty natural to use.

-John