* [PATCH 0/2] some small clarifying docfixes @ 2020-10-16 20:52 Emily Shaffer 2020-10-16 20:52 ` [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency Emily Shaffer 2020-10-16 20:52 ` [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' Emily Shaffer 0 siblings, 2 replies; 14+ messages in thread From: Emily Shaffer @ 2020-10-16 20:52 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency 2020-10-16 20:52 [PATCH 0/2] some small clarifying docfixes Emily Shaffer @ 2020-10-16 20:52 ` Emily Shaffer 2020-10-16 21:21 ` Junio C Hamano 2020-10-16 20:52 ` [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' Emily Shaffer 1 sibling, 1 reply; 14+ messages in thread From: Emily Shaffer @ 2020-10-16 20:52 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency 2020-10-16 20:52 ` [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency Emily Shaffer @ 2020-10-16 21:21 ` Junio C Hamano 2020-10-16 21:52 ` Taylor Blau 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2020-10-16 21:21 UTC (permalink / raw) To: Emily Shaffer; +Cc: git 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency 2020-10-16 21:21 ` Junio C Hamano @ 2020-10-16 21:52 ` Taylor Blau 2020-10-16 22:48 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Taylor Blau @ 2020-10-16 21:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: Emily Shaffer, git 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency 2020-10-16 21:52 ` Taylor Blau @ 2020-10-16 22:48 ` Junio C Hamano 2020-10-22 23:14 ` Emily Shaffer 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2020-10-16 22:48 UTC (permalink / raw) To: Taylor Blau; +Cc: Emily Shaffer, git 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency 2020-10-16 22:48 ` Junio C Hamano @ 2020-10-22 23:14 ` Emily Shaffer 0 siblings, 0 replies; 14+ messages in thread From: Emily Shaffer @ 2020-10-22 23:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Blau, git 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' 2020-10-16 20:52 [PATCH 0/2] some small clarifying docfixes Emily Shaffer 2020-10-16 20:52 ` [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency Emily Shaffer @ 2020-10-16 20:52 ` Emily Shaffer 2020-10-16 21:13 ` Jeff King 2020-10-16 21:53 ` Junio C Hamano 1 sibling, 2 replies; 14+ messages in thread From: Emily Shaffer @ 2020-10-16 20:52 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' 2020-10-16 20:52 ` [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' Emily Shaffer @ 2020-10-16 21:13 ` Jeff King 2020-10-16 22:04 ` Junio C Hamano 2020-10-16 21:53 ` Junio C Hamano 1 sibling, 1 reply; 14+ messages in thread From: Jeff King @ 2020-10-16 21:13 UTC (permalink / raw) To: Emily Shaffer; +Cc: git 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' 2020-10-16 21:13 ` Jeff King @ 2020-10-16 22:04 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2020-10-16 22:04 UTC (permalink / raw) To: Jeff King; +Cc: Emily Shaffer, git 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' 2020-10-16 20:52 ` [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' Emily Shaffer 2020-10-16 21:13 ` Jeff King @ 2020-10-16 21:53 ` Junio C Hamano 2020-10-16 21:59 ` Junio C Hamano ` (2 more replies) 1 sibling, 3 replies; 14+ messages in thread From: Junio C Hamano @ 2020-10-16 21:53 UTC (permalink / raw) To: Emily Shaffer; +Cc: git 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' 2020-10-16 21:53 ` Junio C Hamano @ 2020-10-16 21:59 ` Junio C Hamano 2020-10-16 22:12 ` Junio C Hamano 2020-10-22 23:13 ` Emily Shaffer 2 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2020-10-16 21:59 UTC (permalink / raw) To: Emily Shaffer; +Cc: git 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' 2020-10-16 21:53 ` Junio C Hamano 2020-10-16 21:59 ` Junio C Hamano @ 2020-10-16 22:12 ` Junio C Hamano 2020-10-22 23:13 ` Emily Shaffer 2 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2020-10-16 22:12 UTC (permalink / raw) To: Emily Shaffer; +Cc: git 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' 2020-10-16 21:53 ` Junio C Hamano 2020-10-16 21:59 ` Junio C Hamano 2020-10-16 22:12 ` Junio C Hamano @ 2020-10-22 23:13 ` Emily Shaffer 2020-10-23 3:57 ` Junio C Hamano 2 siblings, 1 reply; 14+ messages in thread From: Emily Shaffer @ 2020-10-22 23:13 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' 2020-10-22 23:13 ` Emily Shaffer @ 2020-10-23 3:57 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2020-10-23 3:57 UTC (permalink / raw) To: Emily Shaffer; +Cc: git 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-10-23 3:57 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-16 20:52 [PATCH 0/2] some small clarifying docfixes Emily Shaffer 2020-10-16 20:52 ` [PATCH 1/2] MyFirstContribution: clarify asciidoc dependency Emily Shaffer 2020-10-16 21:21 ` Junio C Hamano 2020-10-16 21:52 ` Taylor Blau 2020-10-16 22:48 ` Junio C Hamano 2020-10-22 23:14 ` Emily Shaffer 2020-10-16 20:52 ` [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply' Emily Shaffer 2020-10-16 21:13 ` Jeff King 2020-10-16 22:04 ` Junio C Hamano 2020-10-16 21:53 ` Junio C Hamano 2020-10-16 21:59 ` Junio C Hamano 2020-10-16 22:12 ` Junio C Hamano 2020-10-22 23:13 ` Emily Shaffer 2020-10-23 3:57 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).