Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply'

[Junio C Hamano <gitster@pobox.com>]

Emily Shaffer <emilyshaffer@google.com> writes:

> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index 38c0852139..e64f3f10e3 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> @@ -24,6 +24,64 @@ Splits mail messages in a mailbox into commit log message,
>  authorship information and patches, and applies them to the
>  current branch.
>  
> +This command applies patches as commits. Use linkgit:git-apply[1] to apply
> +patches to the worktree without creating commits.

I am not sure if "applies patches as commits" is acceptable,
understandable and not misleading to normal readers.  It takes a
mailbox with mailed patches, and for each message, applies the patch
in it on top of the current commit and records the result as a child
commit using the log message in it.

As "git apply" is primarily meant to work on "git diff" output, and
it does not necessarily work on an arbitrary mbox (think: MIME), I
do not think "if you do not want to make commit, use apply" is a
good suggestion to begin with.  They serve completely different
purposes and take different form of inputs.

> +
> +EXAMPLE
> +-------
> +....
> +# our sample patch, generated with 'git format-patch'
> +$ cat ~/0001-README-add-more-text.patch
> +From f9e01d7538c9d58b48500732b4f98f40f6ad845e Mon Sep 17 00:00:00 2001
> +From: A U Thor <author@example.com>
> +Date: Fri, 16 Oct 2020 13:14:42 -0700
> +Subject: [PATCH] README: add more text
> +
> +Some additional context.
> +
> +Signed-off-by: A U Thor <author@example.com>
> +---
> + README | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/README b/README
> +index cd08755..cfdf4e7 100644
> +--- a/README
> ++++ b/README
> +@@ -1 +1 @@
> +-Hello world!
> ++Hello world! This is an example.
> +--
> +2.29.0.rc1.297.gfa9743e501
> +
> +# use 'git am' to create a new commit with details from the patch
> +$ git status
> +On branch main
> +nothing to commit, working tree clean
> +$ git am ~/0001-README-add-more-text.patch
> +Applying: README: add more text
> +$ git status
> +On branch main
> +nothing to commit, working tree clean
> +$ git log --oneline
> +dd6a400 (HEAD -> main) README: add more text
> +90b59fb base commit

It is good to show "log" output after you injested the patch, but
without knowing that we used to have only one commit in the history,
the gravity of what they are seeing here may not sink in to readers'
minds.

As I said already, I do not agree with the general idea to pretend
that input to am is always a safe input to apply like the
following.  However what we see above as an example session of "am"
is an excellent thing to have, I would think.

> +# use 'git apply' to apply to the worktree without creating a commit.
> +$ git status
> +On branch main
> +nothing to commit, working tree clean
> +$ git apply ~/0001-README-add-more-text.patch

IOW, this step may even fail or produce garbage (think: MIME).

> +$ git status
> +On branch main
> +Changes not staged for commit:
> +  (use "git add <file>..." to update what will be committed)
> +  (use "git restore <file>..." to discard changes in working directory)
> +	modified:   README
> +
> +no changes added to commit (use "git add" and/or "git commit -a")


So, I am moderately against everything under 'use git apply' line of
the patch.  However, I do think it is a good idea to add a note
somewhere in the manual of "am" to say something along the lines of
the following (placed around here, or even immediately before we
give the sample patch we used in the above example):

    While an output of "diff format-patch" (see above/below for an
    example) is meant to be made into a commit with "git am",
    what you have may only be an output of "git diff" without log
    message and is not meant to be directly made into a commit.  In
    such a case, you may want to refer to git-apply[1] to learn how
    to apply such a change to your working tree (and optionally to
    the index).
   
It would be a good idea to redirect those readers who are looking at
"git am" when (perhaps realizing) they should rather be looking at
"git apply" earlier rather than later, so perhaps taking "see below"
side and giving it as a side-note before the example starts might be
better.

> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
> index 91d9a8601c..38e9d8f713 100644
> --- a/Documentation/git-apply.txt
> +++ b/Documentation/git-apply.txt
> @@ -32,6 +32,61 @@ This command applies the patch but does not create a commit.  Use
>  linkgit:git-am[1] to create commits from patches generated by
>  linkgit:git-format-patch[1] and/or received by email.
>  
> +EXAMPLE
> +-------
> +....
> +# our sample patch, generated with 'git format-patch'

s/format-patch/diff/

I would not prepare the diff to be fed to apply with format-patch
and use it verbatim.  This description is sewing seed to confuse
newbies by doing so.  The "apply to the working tree, think while
staring at the resulting 'git diff' output and decide the next move,
which may include typing 'git commit -a -e'" workflow starts with a
"git diff" output.

If you have format-patch output, you are still better off running
"am [-3]" on a throw-away branch if you want to take the "think
while staring at the diff and decide the next move" approach.  That
may better be added to "git am" documentation, though.

> +$ cat ~/0001-README-add-more-text.patch
> +From f9e01d7538c9d58b48500732b4f98f40f6ad845e Mon Sep 17 00:00:00 2001
> +From: A U Thor <author@example.com>
> +Date: Fri, 16 Oct 2020 13:14:42 -0700
> +Subject: [PATCH] README: add more text
> +
> +Some additional context.
> +
> +Signed-off-by: A U Thor <author@example.com>
> +---
> + README | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/README b/README
> +index cd08755..cfdf4e7 100644
> +--- a/README
> ++++ b/README
> +@@ -1 +1 @@
> +-Hello world!
> ++Hello world! This is an example.
> +--
> +2.29.0.rc1.297.gfa9743e501

So, the above sample input needs to be fixed, without e-mail headers
or potential MIME.

> +# use 'git apply' to apply to the worktree without creating a commit.
> +$ git status
> +On branch main
> +nothing to commit, working tree clean
> +$ git apply ~/0001-README-add-more-text.patch
> +$ git status
> +On branch main
> +Changes not staged for commit:
> +  (use "git add <file>..." to update what will be committed)
> +  (use "git restore <file>..." to discard changes in working directory)
> +	modified:   README
> +
> +no changes added to commit (use "git add" and/or "git commit -a")

Or "apply --index" to affect both.  If we are helping people to
learn the command by examples, we should add it (you may only be
interested in teaching the difference between am and apply with this
patch, but readers' interests are not limited by yours).

> +# use 'git am' to create a new commit with details from the patch

Again, I do not think this belongs here, especially given that the
expected input is not necessarily out of format-patch.  Instead, a
similar and opposite referral is needed to help readers who are here
by mistake and should instead be over there.

Thanks.