[PATCH 0/2] some small clarifying docfixes

[PATCH 0/2] some small clarifying docfixes

[Emily Shaffer]

Probably didn't need to string these together but they're both pretty
small. I can separate them if needed.

Emily Shaffer (2):
  MyFirstContribution: clarify asciidoc dependency
  (Just reorders one note.)
  docs: demonstrate difference between 'am' and 'apply'
  (Adds a snippet demonstrating the effects of 'git apply some.patch' vs
  'git am some.patch')

 Documentation/MyFirstContribution.txt |  5 ++-
 Documentation/git-am.txt              | 58 +++++++++++++++++++++++++++
 Documentation/git-apply.txt           | 55 +++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 2 deletions(-)

-- 
2.28.0.rc0.142.g3c755180ce-goog

[PATCH 1/2] MyFirstContribution: clarify asciidoc dependency

[Emily Shaffer]

Per IRC:

[19:52] <lkmandy> With respect to the MyFirstContribution tutorial, I
will like to suggest this - Under the section "Adding Documentation",
just before the "make all doc" command, it will be really helpful to
prompt a user to check if they have the asciidoc package installed, if
they don't, the command should be provided or they can just be pointed
to install it

So, let's move the note about the dependency to before the build command
blockquote.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/MyFirstContribution.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index 4f85a089ef..7522387ae1 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -507,6 +507,9 @@ documentation is consistent with other Git and UNIX manpages; this makes life
 easier for your user, who can skip to the section they know contains the
 information they need.
 
+NOTE: Before trying to build the docs, make sure you have the package `asciidoc`
+installed.
+
 Now that you've written your manpage, you'll need to build it explicitly. We
 convert your AsciiDoc to troff which is man-readable like so:
 
@@ -522,8 +525,6 @@ $ make -C Documentation/ git-psuh.1
 $ man Documentation/git-psuh.1
 ----
 
-NOTE: You may need to install the package `asciidoc` to get this to work.
-
 While this isn't as satisfying as running through `git help`, you can at least
 check that your help page looks right.
 
-- 
2.28.0.rc0.142.g3c755180ce-goog

Re: [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency

[Junio C Hamano]

Emily Shaffer <emilyshaffer@google.com> writes:

> Per IRC:
>
> [19:52] <lkmandy> With respect to the MyFirstContribution tutorial, I
> will like to suggest this - Under the section "Adding Documentation",
> just before the "make all doc" command, it will be really helpful to
> prompt a user to check if they have the asciidoc package installed, if
> they don't, the command should be provided or they can just be pointed
> to install it
>
> So, let's move the note about the dependency to before the build command
> blockquote.

Suggesting asciidoc alone may not be all that helpful, or unless it
drags xmlto, docbook and friends as its dependencies, and the
details of that depends on distro packaging.

As this is only moving the existing note around in the
documentation, it is not making things any worse, so I am OK to take
the patch as-is, but if somebody (it is fine if it were done by you,
Emily) can double check "apt-get install asciidoc" on a vanilla box
does bring in what we need, that would be quite good.  FWIW, we
write in our top-level INSTALL file that we require asciidox/xmlto
toolchain (the latter is needed if you format for manpage, i.e. if
you do "git subcmd --help").

Thanks

Re: [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency

[Taylor Blau]

On Fri, Oct 16, 2020 at 02:21:53PM -0700, Junio C Hamano wrote:
> As this is only moving the existing note around in the
> documentation, it is not making things any worse, so I am OK to take
> the patch as-is, but if somebody (it is fine if it were done by you,
> Emily) can double check "apt-get install asciidoc" on a vanilla box
> does bring in what we need, that would be quite good.  FWIW, we
> write in our top-level INSTALL file that we require asciidox/xmlto
> toolchain (the latter is needed if you format for manpage, i.e. if
> you do "git subcmd --help").

I was curious myself, and surprised to learn that 'apt-get install
asciidoc' installs more than 2GB of dependencies. Yikes. Unsurprisingly,
somewhere in those 2GB we manage to fit in everything that seems to
matter, since:

  $ cat /etc/os-release | grep PRETTY
  PRETTY_NAME="Debian GNU/Linux 9 (stretch)"
  $ apt-get update && apt-get install asciidoc
  $ cd copy-of-git
  $ make -C doc

...works just fine.

Thanks,
Taylor

Re: [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> On Fri, Oct 16, 2020 at 02:21:53PM -0700, Junio C Hamano wrote:
>> As this is only moving the existing note around in the
>> documentation, it is not making things any worse, so I am OK to take
>> the patch as-is, but if somebody (it is fine if it were done by you,
>> Emily) can double check "apt-get install asciidoc" on a vanilla box
>> does bring in what we need, that would be quite good.  FWIW, we
>> write in our top-level INSTALL file that we require asciidox/xmlto
>> toolchain (the latter is needed if you format for manpage, i.e. if
>> you do "git subcmd --help").
>
> I was curious myself, and surprised to learn that 'apt-get install
> asciidoc' installs more than 2GB of dependencies. Yikes. Unsurprisingly,
> somewhere in those 2GB we manage to fit in everything that seems to
> matter, since:
>
>   $ cat /etc/os-release | grep PRETTY
>   PRETTY_NAME="Debian GNU/Linux 9 (stretch)"
>   $ apt-get update && apt-get install asciidoc
>   $ cd copy-of-git
>   $ make -C doc
>
> ...works just fine.

OK, that makes me feel even better.

Thanks.

Re: [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency

[Emily Shaffer]

On Fri, Oct 16, 2020 at 03:48:39PM -0700, Junio C Hamano wrote:
> 
> Taylor Blau <me@ttaylorr.com> writes:
> 
> > On Fri, Oct 16, 2020 at 02:21:53PM -0700, Junio C Hamano wrote:
> >> As this is only moving the existing note around in the
> >> documentation, it is not making things any worse, so I am OK to take
> >> the patch as-is, but if somebody (it is fine if it were done by you,
> >> Emily) can double check "apt-get install asciidoc" on a vanilla box
> >> does bring in what we need, that would be quite good.  FWIW, we
> >> write in our top-level INSTALL file that we require asciidox/xmlto
> >> toolchain (the latter is needed if you format for manpage, i.e. if
> >> you do "git subcmd --help").
> >
> > I was curious myself, and surprised to learn that 'apt-get install
> > asciidoc' installs more than 2GB of dependencies. Yikes. Unsurprisingly,
> > somewhere in those 2GB we manage to fit in everything that seems to
> > matter, since:
> >
> >   $ cat /etc/os-release | grep PRETTY
> >   PRETTY_NAME="Debian GNU/Linux 9 (stretch)"
> >   $ apt-get update && apt-get install asciidoc
> >   $ cd copy-of-git
> >   $ make -C doc
> >
> > ...works just fine.
> 
> OK, that makes me feel even better.

Thanks for checking it, Taylor.
 - Emily

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

[Emily Shaffer]

Some users skimmed past the note in 'git help apply' mentioning 'git
am' and weren't sure how to retain commit details. An example
illustrating the difference between the two shows this information in
another way, so users who prefer to be shown rather than told can
discover the difference too.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---

Notes:
    In this change, I wasn't sure about a few things:
    
    - Should the comments on the snippets live outside of the blockquote instead?
    - Should 'git reset' be included in the snippets, so that users don't try to
      paste without thinking?
    - Should the example go underneath the options list?
    
    Anyway, we got internal feedback that the description in 'git help am' wasn't
    very noticeable. I'm not sure I agree, but it's true that some folks grok
    examples more easily than they grok prose, so we figured it probably couldn't
    hurt to provide both.
    
     - Emily

 Documentation/git-am.txt    | 58 +++++++++++++++++++++++++++++++++++++
 Documentation/git-apply.txt | 55 +++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)

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.
+
+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
+
+# 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")
+....
+
 OPTIONS
 -------
 (<mbox>|<Maildir>)...::
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'
+$ 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 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")
+
+# 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
+....
+
 OPTIONS
 -------
 <patch>...::
-- 
2.28.0.rc0.142.g3c755180ce-goog

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

[Jeff King]

On Fri, Oct 16, 2020 at 01:52:32PM -0700, Emily Shaffer wrote:

> Some users skimmed past the note in 'git help apply' mentioning 'git
> am' and weren't sure how to retain commit details. An example
> illustrating the difference between the two shows this information in
> another way, so users who prefer to be shown rather than told can
> discover the difference too.

Having an example showing the difference seems reasonable. However, I
think it should come lower in the file. Putting such a big chunk between
the DESCRIPTION and OPTIONS sections will makes it much harder to find
the options (and that's what people are usually looking for).

Plus it's customary to have the example section (which is usually
pluralized as EXAMPLES) near the end. See:

  https://man7.org/linux/man-pages/man7/man-pages.7.html

which gives recommended names and order.

> Notes:
>     In this change, I wasn't sure about a few things:
>     
>     - Should the comments on the snippets live outside of the blockquote instead?

I think it is OK as-is, but it might be easier to follow visually with
text like:

   Here's a sample patch that we'll use below.

   -----
   $ cat the-patch, etc
   -----


   We can apply it with `git am`, which will create a new commit:

   -----
   $ git am ...etc...
   -----

Especially in the HTML version, which puts the code blocks into a box
with a different backgrounds, that makes it easy to visually jump to
each box without having to carefully read the contents.

>     - Should 'git reset' be included in the snippets, so that users don't try to
>       paste without thinking?

I don't think they could meaningfully paste this anyway, since they
don't have the base commit that it would apply to (you could make the
patch strict add, but I think it's perhaps desirable that they aren't
real runnable commands).

>     - Should the example go underneath the options list?

Yes. :)

You can say "see the EXAMPLES section below" next to the added text if
you want to point out that the example clarifies it further (though I
don't think it's strictly necessary).

>     Anyway, we got internal feedback that the description in 'git help am' wasn't
>     very noticeable. I'm not sure I agree, but it's true that some folks grok
>     examples more easily than they grok prose, so we figured it probably couldn't
>     hurt to provide both.

I think the example is a reasonable one to show, even aside from the "is
it noticeable" level. Having it present twice feels a little funny, but
cross-referencing ("see the examples section in git-am(1)") gets
awkward.

I do like the added text that back-references "git apply" from the "git
am" manpage.

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

I know you're trying to show with "git status" that the working tree is
kept clean. But it does make the example much longer. I kind of think
showing just "git am" and "git log" would be sufficient.

> +# 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")

Showing status afterwards here makes sense. I'm not sure if you need to
show that it's clean before-hand, though, which could also reduce the
size of the example.

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think it is OK as-is, but it might be easier to follow visually with
> text like:
>
>    Here's a sample patch that we'll use below.
>
>    -----
>    $ cat the-patch, etc
>    -----
>
>
>    We can apply it with `git am`, which will create a new commit:
>
>    -----
>    $ git am ...etc...
>    -----
>
> Especially in the HTML version, which puts the code blocks into a box
> with a different backgrounds, that makes it easy to visually jump to
> each box without having to carefully read the contents.

I agree that presentation-wise the above style is much nicer to
follow.

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

[Junio C Hamano]

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.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> 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

Gaah, too many cycles of wordsmithing and copyediting without the
final proofreading.  The above should read "git format-patch", of
course.

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

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> 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

Again, s/realizing/without &/; Sorry for the noise.

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

Thanks.

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

[Emily Shaffer]

On Fri, Oct 16, 2020 at 02:53:33PM -0700, Junio C Hamano wrote:
> 
> 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.

Okay. I think I use 'git am' to apply individual mails, which also can
be applied with 'git apply'; but yes, 'git am' also can take an mbox,
and 'git apply' can't; 'git apply' can take the output of 'git diff' but
'git am' can't. I wonder how best to say so.

"This command applies patches from email (e.g. the output of 'git
format-patch', or an mbox), preserving metadata and creating commits.
Use <git-apply> to apply patches (e.g. the output of 'git format-patch'
or 'git diff') to the worktree without creating commits."

Thanks for pointing it out. I had blinders on - I use 'git format-patch'
and apply it with whichever is convenient, and in that specific case
either 'git am' or 'git apply' works. I'll try to send a new version
emphasizing the difference in input format.

 - Emily

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

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

[Junio C Hamano]

Emily Shaffer <emilyshaffer@google.com> writes:

>> 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.
>
> Okay. I think I use 'git am' to apply individual mails, which also can
> be applied with 'git apply';

Yes for 'am', sometimes for 'apply' (think: MIME).

> "This command applies patches from email (e.g. the output of 'git
> format-patch', or an mbox), preserving metadata and creating commits.

Yeah, something like that.  

    This command takes a mbox, each message in which is a piece of
    e-mailed patch, which typically is output of `git format-patch`.
    For each message, the patch text is applied to the current
    branch and recorded as a commit, with the authorship information
    and log message taken from the e-mailed message.

> Use <git-apply> to apply patches (e.g. the output of 'git format-patch'
> or 'git diff') to the worktree without creating commits."

Calling 'git format-patch' output 'patches' the same way as 'git
diff' output is inviting confusion.  The plural 'patches' makes
the confusion worse.

    Use `git apply` when you have diff (e.g. `git diff` output) to
    use to modify the working tree files and optionally the index.

Thanks.