Re: [PATCH] notes: mention --notes in more places
Eric Blake 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 > --- > 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=", 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 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 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 ():" (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." 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: mention --notes in more places
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=:: @@ -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[=]:: 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: mention --notes in more places
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 > >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 :-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: mention --notes in more places
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 ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH] notes: mention --notes in more places
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: mention --notes in more places
Jeff King 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] notes: mention --notes in more places
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html