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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux