git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Jeff Hostetler <git@jeffhostetler.com>
Cc: git@vger.kernel.org, gitster@pobox.com,
	Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH v3 0/5] Add --no-ahead-behind to status
Date: Mon, 8 Jan 2018 01:37:12 -0500	[thread overview]
Message-ID: <20180108063712.GB10933@sigill.intra.peff.net> (raw)
In-Reply-To: <2887ad5a-5de9-3c5b-92c3-40b19120e604@jeffhostetler.com>

On Fri, Jan 05, 2018 at 11:46:24AM -0500, Jeff Hostetler wrote:

> > I'm mildly negative on this "level 2" config. If influencing the
> > porcelain via config creates compatibility headaches, then why would we
> > allow it here? And if it doesn't, then why do we need to protect against
> > it? This seems to exist in a funny middle ground that cannot decide
> > whether it is bad or not.
> > 
> > It's like we're inserting a foot-gun, but putting it just far enough out
> > of reach that we can blame the user when they shoot themselves with it.
> > 
> > Is there a compelling use case for this? From the previous discussion,
> > this is the strawman I came up with:
> [...]
> 
> I kinda trying to serve 2 masters here.  As we discussed earlier, we
> don't want config options to change porcelain formats, hence the
> true/false thing only affecting non-porcelain formats.  On the other
> hand, VS and git.exe are on different release schedules.  Normally,
> I'd just have VS emit a "git status --no-ahead-behind --porcelain=v2"
> and be done, but that requires that git.exe gets updated before VS.
> We do control some of that, but if VS gets updated first, that causes
> an error, whereas "git -c status.aheadbehind=<x> status --porcelain=v2"
> does not.  It is respected if/when git is updated and ignored until
> then.  Likewise, if they update git first, we can tell them to set a
> config setting on the repo and inherit it for porcelain v2 output
> without VS knowing about it.  Sorry, if that's too much detail.

OK, so my strawman was totally off-base. :)

That explanation is interesting. I do think you're facing a real problem
with moving the versions in lockstep. But shoe-horning the feature into
config like this seems like a pretty ugly solution:

  1. Then we're stuck with this weird foot-gun config option forever.

  2. It only solves the problem for this one option. Is there a more
     general solution?

The more general solutions I can come up with are:

  1. Is there a way for a caller to query Git to say "do you understand
     --no-ahead-behind?".

     You can ask "git version", but parsing version numbers is
     problematic. We don't have any kind of "feature flags" output, and
     I'm not sure we'd want to get down to the level of specific
     command-line options there.

     One thing you can do is speculatively run "status --no-ahead-behind",
     and if it returns 129, try again without as a fallback. That incurs
     a failed invocation for the fallback case, but it's quick (we fail
     before looking at any data) and you can cache it for the duration
     of a VS session.

  2. There could be some way to tell the option parser that the next
     flag is optional. E.g.:

       git status --optional=--no-ahead-behind

     That would be pretty easy to implement globally in parse-options.c.
     It knows when an option is unrecognized, so it could just treat
     that as a silent noop rather than barfing.

     Of course, that doesn't solve your problem today. It wouldn't be
     safe to start using "--optional" until it had been in several
     released versions.

     And I have a feeling it may not be sufficient without further
     feedback to the caller. For this flag, the caller is happy to say
     "do this if you know how, but otherwise I will cope". But there are
     probably flag where it would need to know whether it had any effect
     or not. So this whole direction is probably crazy.

Of the two, I think (1) is not so bad.

> It is OK with me if we omit the last commit in the patch series (that
> does the experimental =2 extension) and I'll deal with this separately
> (maybe differently) in the gvfs fork.

That would be my preference. Thanks.

-Peff

  parent reply	other threads:[~2018-01-08  6:37 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-03 21:47 [PATCH v3 0/5] Add --no-ahead-behind to status Jeff Hostetler
2018-01-03 21:47 ` [PATCH v3 1/5] stat_tracking_info: return +1 when branches not equal Jeff Hostetler
2018-01-04 21:41   ` Junio C Hamano
2018-01-03 21:47 ` [PATCH v3 2/5] status: add --[no-]ahead-behind to status and commit for V2 format Jeff Hostetler
2018-01-04 22:05   ` Junio C Hamano
2018-01-05 16:31     ` Jeff Hostetler
2018-01-03 21:47 ` [PATCH v3 3/5] status: update short status to respect --no-ahead-behind Jeff Hostetler
2018-01-03 21:47 ` [PATCH v3 4/5] status: support --no-ahead-behind in long format Jeff Hostetler
2018-01-03 21:47 ` [PATCH v3 5/5] status: add status.aheadBehind value for porcelain output Jeff Hostetler
2018-01-04 23:06 ` [PATCH v3 0/5] Add --no-ahead-behind to status Jeff King
2018-01-05 16:46   ` Jeff Hostetler
2018-01-05 19:56     ` Junio C Hamano
2018-01-08  6:37     ` Jeff King [this message]
2018-01-08 14:22       ` Jeff Hostetler

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

  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=20180108063712.GB10933@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).