[PATCH] notes: mention --notes in more places

[PATCH] notes: mention --notes in more places

[Eric Blake]

Every so often, I search 'git send-email --help' to remember some
option I've used in the past, only to discover that the option is
documented instead in 'git format-patch --help'.  Worse, even that
command didn't document the option I was looking for today, which
was how to include 'git notes' in the body of the commits I was
mailing.  Reading 'git notes --help' didn't mention this either,
and I had to resort to searching the source code.  It can't hurt
to add some documentation to make this option less obscure.

* git-notes.txt: Mention that --notes option exists in many
commands to override defaults.
* git-format-patch.txt: Include pretty-options, for things like
--notes.
* git-send-email.txt: Mention that revision lists forwarded to
format-patch can also include options.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 Documentation/git-format-patch.txt | 2 ++
 Documentation/git-notes.txt        | 6 ++++--
 Documentation/git-send-email.txt   | 3 ++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 6d43f56..a068f37 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -222,6 +222,8 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
 	range are always formatted as creation patches, independently
 	of this flag.

+include::pretty-options.txt[]
+
 CONFIGURATION
 -------------
 You can specify extra mail header lines to be added to each message,
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index b95aafa..be9e60f 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -39,8 +39,10 @@ message stored in the commit object, the notes are indented like the
 message, after an unindented line saying "Notes (<refname>):" (or
 "Notes:" for `refs/notes/commits`).

-To change which notes are shown by 'git log', see the
-"notes.displayRef" configuration in linkgit:git-log[1].
+To change which notes are shown by default in 'git log', see the
+"notes.displayRef" configuration in linkgit:git-log[1].  Also,
+many commands understand a `--notes` option to alter the set of
+notes displayed (see linkgit:git-rev-list[1]).

 See the "notes.rewrite.<command>" configuration for a way to carry
 notes across commands that rewrite commits.
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index eeb561c..450d975 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -18,7 +18,8 @@ Takes the patches given on the command line and emails them out.
 Patches can be specified as files, directories (which will send all
 files in the directory), or directly as a revision list.  In the
 last case, any format accepted by linkgit:git-format-patch[1] can
-be passed to git send-email.
+be passed to git send-email, including additional command line
+options such as `--cover-letter` or `--notes`.

 The header of the email is configurable by command line options.  If not
 specified on the command line, the user will be prompted with a ReadLine
-- 
1.7.11.7

Re: [PATCH] notes: mention --notes in more places

[Junio C Hamano]

Eric Blake <eblake@redhat.com> writes:

> * git-notes.txt: Mention that --notes option exists in many
> commands to override defaults.
> * git-format-patch.txt: Include pretty-options, for things like
> --notes.
> * git-send-email.txt: Mention that revision lists forwarded to
> format-patch can also include options.

Overall I feel fairly negative on this one, even though there are
good bits.

>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  Documentation/git-format-patch.txt | 2 ++
>  Documentation/git-notes.txt        | 6 ++++--
>  Documentation/git-send-email.txt   | 3 ++-
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 6d43f56..a068f37 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -222,6 +222,8 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
>  	range are always formatted as creation patches, independently
>  	of this flag.
>
> +include::pretty-options.txt[]

In the context of format-patch, the inclusion of pretty-options
probably causes more harm than being helpful, I am afraid.  If you
use "--pretty=<format>", "--format=<format>", or "--oneline", the
output will no longer be a proper mbox and is not suitable for
asking somebody else to apply.

At the very least, you would need to add something like:

    ifndef::git-format-patch[]
    ... enclose everything that should not be used with format-patch
    endif::git-format-patch[]

to the included file, and then define the token before the
inclusion, like this:

    :git-format-patch: 1
    include::pretty-formats.txt[]

to limit the damage.

Even with such a change to include only --notes, I am not sure if
the result is something we would want to recommend/advertise to our
users.

The output from format-patch with --notes shows the notes, after
adding a blank line to the sign-off block, to look like this:

	From: A U Thor <author@example.com>
        Date: Tue, 16 Oct 2012 19:26:23 +0200
	Subject: [PATCH] Gostak: distim the doshes correctly

	With the current code, the Gostak cannot correctly distim
        the doshes, because ...

	Signed-off-by: Junio C Hamano <gitster@pobox.com>

	Notes:
		This patch was inspired by Eric Blake

	---
	diff --git a/gostak b/gostak
        ...

I am not sure if this is suiable for sending to somebody and asking
it to be applied.

> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index b95aafa..be9e60f 100644
> --- a/Documentation/git-notes.txt
> +++ b/Documentation/git-notes.txt
> @@ -39,8 +39,10 @@ message stored in the commit object, the notes are indented like the
>  message, after an unindented line saying "Notes (<refname>):" (or
>  "Notes:" for `refs/notes/commits`).
>
> -To change which notes are shown by 'git log', see the
> -"notes.displayRef" configuration in linkgit:git-log[1].
> +To change which notes are shown by default in 'git log', see the
> +"notes.displayRef" configuration in linkgit:git-log[1].  Also,
> +many commands understand a `--notes` option to alter the set of
> +notes displayed (see linkgit:git-rev-list[1]).
>
>  See the "notes.rewrite.<command>" configuration for a way to carry
>  notes across commands that rewrite commits.

OK.

> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index eeb561c..450d975 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -18,7 +18,8 @@ Takes the patches given on the command line and emails them out.
>  Patches can be specified as files, directories (which will send all
>  files in the directory), or directly as a revision list.  In the
>  last case, any format accepted by linkgit:git-format-patch[1] can
> -be passed to git send-email.
> +be passed to git send-email, including additional command line
> +options such as `--cover-letter` or `--notes`.

OK for --cover-letter, dubious on --notes.

Re: [PATCH] notes: mention --notes in more places

[Jeff King]

On Tue, Oct 16, 2012 at 09:19:35PM -0600, Eric Blake wrote:

> Every so often, I search 'git send-email --help' to remember some
> option I've used in the past, only to discover that the option is
> documented instead in 'git format-patch --help'.  Worse, even that
> command didn't document the option I was looking for today, which
> was how to include 'git notes' in the body of the commits I was
> mailing.  Reading 'git notes --help' didn't mention this either,
> and I had to resort to searching the source code.  It can't hurt
> to add some documentation to make this option less obscure.

I think this is a good direction, but...

> * git-notes.txt: Mention that --notes option exists in many
> commands to override defaults.
> * git-format-patch.txt: Include pretty-options, for things like
> --notes.

There are many things in pretty-options that would not be appropriate
for format-patch. We should probably wrap them like this:

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 5e49942..a0f1d15 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -1,3 +1,4 @@
+ifndef::git-format-patch[]
 --pretty[=<format>]::
 --format=<format>::
 
@@ -27,6 +28,7 @@ people using 80-column terminals.
 --oneline::
 	This is a shorthand for "--pretty=oneline --abbrev-commit"
 	used together.
+endif::git-format-patch[]
 
 --encoding[=<encoding>]::
 	The commit objects record the encoding used for the log message

It may also make sense to show notes differently when outputting the
"email" format as format-patch does. E.g., using a triple-dash would
keep them separate from the commit message when using "git am". Like:

  your commit message

  Signed-off-by: You
  ---
  your notes go here

We've talked about it several times, but it's never happened (probably
because most people don't actually use notes).

-Peff

Re: [PATCH] notes: mention --notes in more places

[Junio C Hamano]

Jeff King <peff@peff.net> wrote:

>It may also make sense to show notes differently when outputting the
>"email" format as format-patch does. E.g., using a triple-dash would
>keep them separate from the commit message when using "git am". Like:
>
>  your commit message
>
>  Signed-off-by: You
>  ---
>  your notes go here
>
>We've talked about it several times, but it's never happened (probably
>because most people don't actually use notes).

It is sometimes scary how we end up saying identical things independently :-) 

Re: [PATCH] notes: mention --notes in more places

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]

On 10/16/2012 11:51 PM, Jeff King wrote:
> It may also make sense to show notes differently when outputting the
> "email" format as format-patch does. E.g., using a triple-dash would
> keep them separate from the commit message when using "git am". Like:
> 
>   your commit message
> 
>   Signed-off-by: You
>   ---
>   your notes go here

That's _precisely_ what I want!  I want to use notes as a way of
tracking my edits for what I did in v2 of a patch, at the time I commit
my v2, so that I can send a revised series including the notes in a
manner most efficient for someone else using 'git am' on the series to
see why I sent a v2 but without polluting the upstream repository with
useless versioning information from the email.

> 
> We've talked about it several times, but it's never happened (probably
> because most people don't actually use notes).

And people (like me) don't use notes because they aren't documented.
Catch-22, so we have to start somewhere.

I'll submit a v2 with the non-controversial edits, and spend some time
trying to figure out how to isolate the portion of pretty-options.txt
that is relevant to format-patch.  If it's easy enough, I can also
consider using --- instead of Notes: as the separator when using
format-patch.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [PATCH] notes: mention --notes in more places

[Jeff King]

On Wed, Oct 17, 2012 at 07:30:56AM -0600, Eric Blake wrote:

> > We've talked about it several times, but it's never happened (probably
> > because most people don't actually use notes).
> 
> And people (like me) don't use notes because they aren't documented.
> Catch-22, so we have to start somewhere.

Oh, I definitely agree your patch is the right direction. I was just
explaining why it hasn't happened, even though people think it's a good
idea.

> I'll submit a v2 with the non-controversial edits, and spend some time
> trying to figure out how to isolate the portion of pretty-options.txt
> that is relevant to format-patch.  If it's easy enough, I can also
> consider using --- instead of Notes: as the separator when using
> format-patch.

Hmm. After digging in the archive, it seems we (including both you and
me!) have discussed this several times, and there are even some patches
floating around. Maybe one of them would be a good starting point for
your submission (I did not read carefully over all of the arguments for
each):

  Patch from Thomas, Feb 2010:

    http://thread.gmane.org/gmane.comp.version-control.git/139919/focus=140818

  Discussion between us, Dec 2010:

    http://thread.gmane.org/gmane.comp.version-control.git/163141

  Patch from Michael, Apr 2011:

    http://thread.gmane.org/gmane.comp.version-control.git/172079

-Peff

Re: [PATCH] notes: mention --notes in more places

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Oct 17, 2012 at 07:30:56AM -0600, Eric Blake wrote:
>
>> > We've talked about it several times, but it's never happened (probably
>> > because most people don't actually use notes).
>> 
>> And people (like me) don't use notes because they aren't documented.
>> Catch-22, so we have to start somewhere.
>
> Oh, I definitely agree your patch is the right direction. I was just
> explaining why it hasn't happened, even though people think it's a good
> idea.
>
>> I'll submit a v2 with the non-controversial edits, and spend some time
>> trying to figure out how to isolate the portion of pretty-options.txt
>> that is relevant to format-patch.  If it's easy enough, I can also
>> consider using --- instead of Notes: as the separator when using
>> format-patch.
>
> Hmm. After digging in the archive, it seems we (including both you and
> me!) have discussed this several times, and there are even some patches
> floating around. Maybe one of them would be a good starting point for
> your submission (I did not read carefully over all of the arguments for
> each):

Thomas's oldest one looked like a good starting point but we've
gained a codepath to spit out the contents of notes since then, which
probably needs to be killed at least for this codepath.

A few problems I noticed while looking at log-tree.c and pretty.c

 * pretty_print_commit() shows notes at the end of existing
   message.  There is no provision for the callers to affect what
   comes between the existing log message and the notes text.

 * show_log() has the "add-signoff" that appends a sign-off after
   whatever pretty_print_commit() gives.

Taken together, they make it unnecessarily cumbersome to inject a
new sign-off and "---" between the log message and notes.

The easiest is to add another parameter to pretty_print_commit that
is inserted immediately after the log message before notes are
appended.  That way, we can update show_log() to first format
additional sign off (if needed) and then "---\n" (again, if needed)
to a new strbuf and pass it as the new argument when calling the
pretty_print_commit() function.

Re: [PATCH] notes: mention --notes in more places

[Michael J Gruber]

Jeff King venit, vidit, dixit 17.10.2012 21:05:
> On Wed, Oct 17, 2012 at 07:30:56AM -0600, Eric Blake wrote:
> 
>>> We've talked about it several times, but it's never happened (probably
>>> because most people don't actually use notes).
>>
>> And people (like me) don't use notes because they aren't documented.
>> Catch-22, so we have to start somewhere.
> 
> Oh, I definitely agree your patch is the right direction. I was just
> explaining why it hasn't happened, even though people think it's a good
> idea.
> 
>> I'll submit a v2 with the non-controversial edits, and spend some time
>> trying to figure out how to isolate the portion of pretty-options.txt
>> that is relevant to format-patch.  If it's easy enough, I can also
>> consider using --- instead of Notes: as the separator when using
>> format-patch.
> 
> Hmm. After digging in the archive, it seems we (including both you and
> me!) have discussed this several times, and there are even some patches
> floating around. Maybe one of them would be a good starting point for
> your submission (I did not read carefully over all of the arguments for
> each):
> 
>   Patch from Thomas, Feb 2010:
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/139919/focus=140818
> 
>   Discussion between us, Dec 2010:
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/163141
> 
>   Patch from Michael, Apr 2011:
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/172079

That one used to work for about one more year or so (it went through a
few rebases) but stopped working during some rework involving the
signature (signed-off-by), i.e. it puts the notes before the signed-off
now. I didn't update it because nobody seemed interested anyway (and
because branch-notes got implemented in a different, non-note way, so I
dumped that part of my workflow also).

Michael