Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame

From: Luben Tuikov <ltuikov@yahoo.com>
Date: Jun 3, 2008, 20:35 UTC
Message-ID: <940824.46903.qm@web31808.mail.mud.yahoo.com>
In-Reply-To: <b77c1dce0806030600x520d35edxbe6e732ce6cc4ad6@mail.gmail.com>

--- On Tue, 6/3/08, Rafael Garcia-Suarez <rgarciasuarez@gmail.com> wrote:

Show 24 quoted lines

> From: Rafael Garcia-Suarez <rgarciasuarez@gmail.com>
> Subject: Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame
> To: "Jakub Narebski" <jnareb@gmail.com>
> Cc: git@vger.kernel.org, "Luben Tuikov" <ltuikov@yahoo.com>
> Date: Tuesday, June 3, 2008, 6:00 AM
> 2008/6/3 Jakub Narebski <jnareb@gmail.com>:
> >>> I'd rather remove this, correct it, or
> make it optional (this is very
> >>> fork-heavy).
> >>
> >> Not sure how to do the same thing in pure Perl.
> >
> > I was thinking about extending git-blame porcelain
> format (and also
> > incremental format, of course) by 'parents'
> (and perhaps
> > 'original-parents') header...
> 
> OK, I see. That would be nice. Also: currently taking
> "$full_rev^"
> directs the user to the parent commit, but it would be more
> user-friendly to point at the previous commit where the
> selected file
> was modified instead.

The intention was that it shouldn't necessarily be the (strict) parent of the change (changed segment), since it may or may not have changed in the strict parent commit. The intention was that it "starts"/"opens" the parent commit so that "git" would start from there and find the actual change/commit where that line/segment has changed. And it has worked pretty fine for me when data-mining (something I do quite often) code evolution.

My commit 244a70e608204a515c214a11c43f3ecf7642533a was really derived from a command line, which I had started to use quite often and had been "looking for" for quite some time.

Show 15 quoted lines

> >> We could however cache the results of
> git-rev-parse, since the same
> >> rev is likely to appear many times in the list.
> >
> > ...but starting with cache of git-rev-parse results,
> or optionally
> > allowing extended sha-1 syntax (including
> <hash>^) in hash* CGI
> > parameters in gitweb would be a good idea.
> >
> > But as I wrote, I'm fine with the patch as it is
> now.
> 
> I've sent a new version (take 2) with caching. And
> comments, as Lea suggested :)

Yes, hashing is good if it speeds up lookups without altering intended functionality.

Thanks everyone!
    Luben