git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Johan Herland <johan@herland.net>
Cc: Michael J Gruber <git@drmicha.warpmail.net>,
	git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] revision.c: introduce --notes-ref= to use one notes ref only
Date: Tue, 29 Mar 2011 10:33:57 -0400	[thread overview]
Message-ID: <20110329143357.GA10771@sigill.intra.peff.net> (raw)
In-Reply-To: <201103291439.17197.johan@herland.net>

On Tue, Mar 29, 2011 at 02:39:17PM +0200, Johan Herland wrote:

> On Tuesday 29 March 2011, Michael J Gruber wrote:
> > As notes become increasingly popular, it's often interesting to show
> > notes from a particular notes ref only. Introduce '--notes-ref=<ref>'
> > as a convenience shortcut for '--no-standard-notes
> > --show-notes=<ref>'.
> >
> > Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> > ---
> > The idea is to use the same name as in "git notes --ref=<ref>" but
> > make it clear for the rev-list option to be about notes, thus
> > "--notes-ref=<ref>".
> 
> The idea and implementation look good to me. Not sure I like the 
> option "bloat" (somehow feels it should be possible to express the same 
> behavior using fewer options), but if there's not a better way to 
> reorganize the options, then you can consider it Acked-by me.

I feel this would be more consistent with most other options that take
an optional argument:

  1. "--show-notes" uses default refs

  2. "--show-notes=<ref>" shows _just_ <ref>, no defaults

  3. "--show-notes=<ref1> --show-notes=<ref2>" shows <ref1> and <ref2>

  4. (Probably) "--show-notes --show-notes=<ref>" should show default
     refs and <ref>. This is the one I'm least sure of, as it leaves no
     way to override what came earlier on the command line (which is
     useful if, for example, we end up with Michael's proposed ui.log).
     Perhaps "--no-notes" would reset, so:

       --show-notes --no-notes --show-notes=<ref>

     would be equivalent to:

       --show-notes=<ref>

Of course a total behavior change of what --show-notes currently does.

Speaking of which, it is kind of weird that --show-notes is negated by
--no-notes. So maybe it makes sense to introduce "--notes[=<ref>]" to do
what I wrote above, and deprecate --show-notes.

-Peff

  reply	other threads:[~2011-03-29 14:34 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-29 10:05 [PATCH] revision.c: introduce --notes-ref= to use one notes ref only Michael J Gruber
2011-03-29 12:39 ` Johan Herland
2011-03-29 14:33   ` Jeff King [this message]
2011-03-29 15:16     ` Michael J Gruber
2011-03-29 18:32     ` Junio C Hamano
2011-03-29 20:53       ` Jeff King
2011-03-29 20:55         ` [PATCH 1/6] notes: make expand_notes_ref globally accessible Jeff King
2011-03-29 20:56         ` [PATCH 2/6] revision.c: refactor notes ref expansion Jeff King
2011-03-29 20:56         ` [PATCH 3/6] notes: refactor display notes extra refs field Jeff King
2011-03-29 20:57         ` [PATCH 4/6] notes: refactor display notes default handling Jeff King
2011-03-29 22:02           ` Junio C Hamano
2011-03-29 20:57         ` [PATCH 5/6] revision.c: support --notes command-line option Jeff King
2011-03-29 20:59         ` [PATCH 6/6] revision.c: make --no-notes reset --notes list Jeff King
2011-03-29 22:04           ` Junio C Hamano
2011-03-29 21:44         ` [PATCH] revision.c: introduce --notes-ref= to use one notes ref only Johan Herland
2011-03-29 22:18           ` [PATCH 7/6] log/pretty-options: Document --[no-]notes and deprecate old notes options Johan Herland
2011-03-30  0:22             ` Jeff King
2011-03-30  0:57               ` Johan Herland
2011-03-30  3:32                 ` Jeff King
2011-03-29 14:35 ` [PATCH] revision.c: introduce --notes-ref= to use one notes ref only Jeff King
2011-03-29 19:01   ` Jeff King
2011-03-29 19:48     ` Michael J Gruber
2011-03-29 20:23       ` Jeff King

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

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

  git send-email \
    --in-reply-to=20110329143357.GA10771@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johan@herland.net \
    /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.
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).