From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.3 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD, STOX_REPLY_TYPE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id 11A1E20A04 for ; Mon, 17 Apr 2017 20:59:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753914AbdDQU7t (ORCPT ); Mon, 17 Apr 2017 16:59:49 -0400 Received: from smtp-out-2.talktalk.net ([62.24.135.66]:22058 "EHLO smtp-out-2.talktalk.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752557AbdDQU7r (ORCPT ); Mon, 17 Apr 2017 16:59:47 -0400 Received: from PhilipOakley ([92.31.218.76]) by smtp.talktalk.net with SMTP id 0DkOdSSno46SJ0DkOdiOev; Mon, 17 Apr 2017 21:59:45 +0100 X-Originating-IP: [92.31.218.76] X-Spam: 0 X-OAuthority: v=2.2 cv=CItoZljD c=1 sm=1 tr=0 a=e6L6E7eW+5Nb7SO+DvSdIg==:117 a=e6L6E7eW+5Nb7SO+DvSdIg==:17 a=IkcTkHD0fZMA:10 a=pGLkceISAAAA:8 a=iNp9GaFfUnP6MSue7l0A:9 a=QEXdDO2ut3YA:10 a=6kGIvZw6iX1k4Y-7sg4_:22 Message-ID: <5FD0803E166B4D2F9F64D8D21AC23EB3@PhilipOakley> Reply-To: "Philip Oakley" From: "Philip Oakley" To: "Christoph Michelbach" Cc: "Git Mailing List" References: <1492287435.14812.2.camel@gmail.com> <9535BE255A654CADB7B0AE7599A6FA96@PhilipOakley> <1492347718.19687.14.camel@gmail.com> <2DCA89C3FDFF41E5B3651018BF837267@PhilipOakley> <1492368692.22852.9.camel@gmail.com> <1492380399.19991.13.camel@gmail.com> <5EBADDE444D141918F6873BE8456E026@PhilipOakley> <1492452173.11708.22.camel@gmail.com> Subject: Re: [PATCH] Documentation/git-checkout: make doc. of checkout clearer Date: Mon, 17 Apr 2017 21:59:44 +0100 Organization: OPDS MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset="UTF-8"; reply-type=original Content-Transfer-Encoding: 7bit X-Priority: 3 X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook Express 6.00.2900.5931 X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.6157 X-CMAE-Envelope: MS4wfEvRTPClSvB5QjJuIjunCH3HEubnp9uCr3P5N+qckAr4Yj5L+bF7g/CvFMj1U8EPN4+Fv4zL0QPsXJLYePABpdH6G3/mc1/9sGNvoYHHB6iyAvEH5v9v U9YeT3VpCEg+uzvk2s6PviU+WPiGLzyzGTY3+koK3UGnYusvVjBO4MiU3PQofE1QTKcb2y3CvwQpmoZaResnYmbOjWjwROOWfZw= Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org I've added back the list, as it was accidentally dropped. From: "Christoph Michelbach" > On Mon, 2017-04-17 at 17:04 +0100, Philip Oakley wrote: >> From: "Christoph Michelbach" >> > >> > On Sun, 2017-04-16 at 22:25 +0100, Philip Oakley wrote: >> > > >> > > From: "Christoph Michelbach" >> > > > >> > > > It's: git checkout [-p|--patch] [] [--] ... >> > > 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 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 (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 -- ) case we don't get >> that >> two step option. There is IIUC no 'git copyout -- ' >> 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 [] 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 `...` > 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 `...` 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