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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 24, 2017, 01:55 UTC
Message-ID: <xmqqo9vm36xg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1492881136.7368.3.camel@gmail.com>

Christoph Michelbach <michelbach94@gmail.com> writes:

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

Show 10 quoted lines

> From: Christoph Michelbach <michelbach94@gmail.com>
> 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/

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

Missing full-stop after "tree".

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

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

Show 17 quoted lines

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

> +	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.

> +...	Note that because the index is updated, the
> +	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"?