git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] docs: demonstrate difference between 'am' and 'apply'
Date: Fri, 16 Oct 2020 17:13:27 -0400	[thread overview]
Message-ID: <20201016211327.GC3356073@coredump.intra.peff.net> (raw)
In-Reply-To: <20201016205232.2546562-3-emilyshaffer@google.com>

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

  reply	other threads:[~2020-10-16 21:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201016211327.GC3356073@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).