Web lists-archives.com

Re: [PATCH v3 0/5] Add --no-ahead-behind to status






On 1/8/2018 1:37 AM, Jeff King wrote:
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


Interesting ideas, but probably overkill for now.  I'll pull it out
of my next version and deal with it differently our gvfs fork.

Thanks,
Jeff