Re: [RFC/PATCH] shortstatus v1

From: Jeff King <peff@peff.net>
Date: Feb 10, 2009, 13:01 UTC
Message-ID: <20090210130153.GA17305@coredump.intra.peff.net>
In-Reply-To: <499174E8.3030207@drmicha.warpmail.net>

On Tue, Feb 10, 2009 at 01:36:56PM +0100, Michael J Gruber wrote:

Show 5 quoted lines

> > I would much prefer that, if it had been done that way from the
> > beginning. But I think we are stuck with "git status" due to hysterical
> > raisins.
> 
> ROTFTCOOTF!

I have to admit, this is a new acronym for me...I get the first half, but not the second.

Show 10 quoted lines

> > The big downside with the snippet I posted above is that it runs three
> > separate commands that go through the index. In theory, you could do it
> > in one pass. But wt-status _doesn't_ do that, since the diff
> > infrastructure isn't there (a long time ago, Junio had an experimental
> > parallel diff walker patch, but it never made it out of next).
> 
> We completely agree. How do you suggest to progress? Go for the diff
> walker? For a (porc.) command like shortstatus I think going through the
> index 3 times isn't that bad, all disk access should be cached after the
> first run.

I don't know if resurrecting the parallel diff walker is worth the trouble. I guess if somebody cares enough about the performance they can find the old patch and try benchmarking it.

Making a C command rather than a shell script is probably reasonable if this is performance critical (and there seems to be talk of putting it into a prompt). What I really object to in the patch is:

  - sticking this in builtin-commit.c. It really has _nothing_ to do
    with commit or the existing status. Especially using the same
    option parser is just nonsensical.

  - I'm not sure bolting this onto wt-status really makes much sense.
    Especially the performance-critical --mini prompt mode _doesn't_
    want to do the string collection because it wastes a lot of cycles
    figuring out things that we are just going to throw away.

    However, for the "regular" mode, I don't think it is too big a
    problem. wt_status does a few extra things that shortstatus won't
    care about, but I don't think they are too performance critical
    (e.g., I believe it will find out the "your branch is N commits
    ahead of the remote" information).

-Peff