git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Jeff King <peff@peff.net>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH] rev-list: clarify --abbrev and --abbrev-commit usage
Date: Fri, 14 Jun 2019 17:27:14 -0400
Message-ID: <20190614212714.GA15798@sigill.intra.peff.net> (raw)
In-Reply-To: <20190614205950.GC233791@google.com>

On Fri, Jun 14, 2019 at 01:59:50PM -0700, Emily Shaffer wrote:

> > So no, I cannot see a way in which "rev-list --abbrev" is useful without
> > "--abbrev-commit". Which means that perhaps the former should imply the
> > latter.
> 
> Maybe it should; maybe this patch is a good excuse to do so, and enforce
> mutual exclusion of --abbrev-commit/--abbrev and --no-abbrev. Maybe it's
> also a good time to add --abbrev-commit=<length>?

Hmm, yeah, I think that would reduce confusion quite a bit. If it were
"--abbrev-commit=<length>", then "--abbrev" would not be useful for
anything in rev-list. It would still work as a historical item, but we
would not need or want to advertise it in the usage at all. Good
suggestion.

> > is not right. Possibly:
> > 
> >   --abbrev-commit [--abbrev=<n>] | --no-abbrev
> > 
> > would show the interaction more clearly, but I don't have a strong
> > opinion.
> 
> I did consider demonstrating it this way, but when both --abbrev-commit
> and --no-abbrev are used together, we don't complain or reject the
> invocation - which I would expect if the usage states the two options
> are mutually exclusive.

Ah, I see. I don't consider "|" to indicate an exclusion to the point
that the options are rejected. Only that you wouldn't want to use both,
because one counteracts the other. So every "--no-foo" is mutually
exclusive with "--foo" in the sense that one override the other. But the
outcome is "last one wins", and not "whoops, we cannot figure out what
you meant". And that's what the original:

      --abbrev=<n> | --no-abbrev

before your patch was trying to say (and I suspect there are many other
cases of "|" with this kind of last-one-wins behavior).

> I've been trying to think of good reasons not to enforce their mutual
> exclusion, and the one I keep coming back to is that --no-abbrev might
> be desired to override a git config'd abbreviation length - although I
> didn't check to see whether we have one, maybe we would want one later.
> And even in that case, I suppose that --abbrev-commit would not be
> explicitly added to the call (because we'd infer from the config), or
> that if it did need to be explicitly added (like if we need the user to
> say they want abbreviation, but we want to use their configured
> preferred length) then we could still reject the addition of
> --no-abbrev.
>
> So maybe it makes even more sense to take this patch as an opportunity
> to make these options mutually exclusive... although that checking I
> think would wind up in revision.c, and therefore widen the impact of
> the change significantly.

You can configure core.abbrev, though I'm not sure if it ever requests
abbreviation itself, or if it simply sets the length when we do happen
to abbreviate based on command-line options.

But forgetting config for a moment, last-one-wins is useful even among
command line options. E.g., imagine an alias like this:

  [alias]
  mylog = git rev-list --abbrev-commit --pretty=oneline

It's nice if you can run "git mylog --no-abbrev" and have it do what you
expect, instead of complaining "you cannot use --abbrev-commit and
--no-abbrev together".

That's a toy example, but you can imagine more elaborate scripts that
set some default options, and allow arbitrary per-invocation options to
be appended.

-Peff

  reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13 22:15 Emily Shaffer
2019-06-14 16:09 ` Junio C Hamano
2019-06-14 16:18 ` Jeff King
2019-06-14 20:59   ` Emily Shaffer
2019-06-14 21:27     ` Jeff King [this message]
2019-06-14 22:56       ` Emily Shaffer

Reply instructions:

You may reply publically 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=20190614212714.GA15798@sigill.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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror http://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox