threads / patch / 31843

patchnotes: mention --notes in more places

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

## tl;dr

8 messages between Oct 17, 2012 and Oct 18, 2012. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

Eric Blake· Oct 17, 2012, 03:19 UTC · lore

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(-)
Show changes to 3 files +8 −3

Documentation/git-format-patch.txt, Documentation/git-notes.txt, Documentation/git-send-email.txt

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
Junio C Hamano· Oct 17, 2012, 05:14 UTC · re: Eric Blake · lore

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

Eric Blake <eblake@redhat.com> writes:
Show 6 quoted lines
> * 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.

Show 17 quoted lines
>
> 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.

Show 17 quoted lines
> 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.
Show 11 quoted lines
> 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.
Jeff King· Oct 17, 2012, 05:51 UTC · re: Eric Blake · lore

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

On Tue, Oct 16, 2012 at 09:19:35PM -0600, Eric Blake wrote:
Show 8 quoted lines
> 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:

Show changes to Documentation/pretty-options.txt +2 −1
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
Junio C Hamano· Oct 17, 2012, 06:25 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> wrote:
Show 12 quoted lines
>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 :-) 
Eric Blake· Oct 17, 2012, 13:30 UTC · re: Jeff King · lore

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

On 10/16/2012 11:51 PM, Jeff King wrote:
Show 9 quoted lines
> 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
Jeff King· Oct 17, 2012, 19:05 UTC · re: Eric Blake · lore

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

On Wed, Oct 17, 2012 at 07:30:56AM -0600, Eric Blake wrote:
Show 5 quoted lines
> > 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.

Show 5 quoted lines
> 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
Junio C Hamano· Oct 17, 2012, 21:50 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> writes:
Show 23 quoted lines
> 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.

Michael J Gruber· Oct 18, 2012, 12:11 UTC · re: Jeff King · lore

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

Jeff King venit, vidit, dixit 17.10.2012 21:05:
Show 35 quoted lines
> 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

← back to recent threads