From: Philip Oakley <philipoakley@iee.org>
Date: Sun, 16 Apr 2017 21:25:56 GMT
Subject: Re: [PATCH] Documentation/git-checkout: make doc. of checkout <tree-ish> clearer
Message-ID: <DF5E72F5BD2F4BB99D8EC4DF1B4543F7@PhilipOakley>
In-Reply-To: <1492368692.22852.9.camel@gmail.com>

From: "Christoph Michelbach" <michelbach94@gmail.com>
> On Sun, 2017-04-16 at 19:03 +0100, Philip Oakley wrote:
>> From: "Christoph Michelbach" <michelbach94@gmail.com>
>> >
>> > On Sun, 2017-04-16 at 00:28 +0100, Philip Oakley wrote:
>> > >
>> > > From: "Christoph Michelbach" <michelbach94@gmail.com>
>> > > >
>> > > >
>> > > > 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.
>> > > >
>> > > > Signed-off-by: Christoph Michelbach <michelbach94@gmail.com>
>> > > > ---
>> > > > Documentation/git-checkout.txt | 7 ++++---
>> > > > 1 file changed, 4 insertions(+), 3 deletions(-)
>> > > >
>> > > > diff --git a/Documentation/git-checkout.txt
>> > > > b/Documentation/git-checkout.txt
>> > > > index 8e2c066..cfd7b18 100644
>> > > > --- a/Documentation/git-checkout.txt
>> > > > +++ b/Documentation/git-checkout.txt
>> > > > @@ -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
>> > > 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.
>> 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.
>
> Ah, right. I added a missing "to" and changed "to specify a specific".
>
>
>> > > 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.
>
> 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.

>
> 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.
>
> Signed-off-by: Christoph Michelbach <michelbach94@gmail.com>
> ---
> Documentation/git-checkout.txt | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-
> checkout.txt
> index 8e2c066..cfd7b18 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -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

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

> - (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
>