Re: [PATCH] gitweb: Use git-show-ref instead of git-peek-remote

From: Jakub Narebski <jnareb@gmail.com>
Date: Nov 24, 2006, 22:35 UTC
Message-ID: <200611242335.15964.jnareb@gmail.com>
In-Reply-To: <7vhcwoa3mx.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Use "git show-ref --dereference" instead of "git peek-remote ." in

It should be 
   Use "git show-ref --dereference" instead of "git peek-remote <project>"

Show 5 quoted lines

>> git_get_references. git-show-ref is faster than git-peek-remote; even
>> faster is reading info/refs file (if it exists), but the information
>> in info/refs can be stale.
> 
> More importantly, it is for dumb protocol transports, not for gitweb.

Yes, but if it is there, why not make use of it? Especially that it is used only (I think) for refs markers, and not for anything important. On the other side a bit of performance here doesn't matter much.

> You forgot to mention that you fixed the last place that
> directly used "$GIT" to invoke the command, bypassing sub
> git_cmd.  That is a consistency clean-up worth mentioning.

This is not fix, this is change of style. The style was to use
  git peek-remote $projectroot/$project
instead of equivalent
  git --git-dir=$projectroot/$project peek-remote .

We don't have this choice with git-show-ref.

On the other hand that makes all command invocation (except the one used for getting git core version) pass through git_cmd() subroutine.

Show 11 quoted lines

>> git-show-ref is available since v1.4.4; the output format is slightly
>> different than git-peek-remote output format.
> 
>> -		if ($line =~ m/^([0-9a-fA-F]{40})\trefs\/($type\/?[^\^]+)/) {
>> +		if ($line =~ m/^([0-9a-fA-F]{40}) refs\/($type\/?[^\^]+)/) {
> 
> I would rather do:
> 
> 	m|^([0-9a-f]{40})\srefs/($type/?[^^]+)|
> 
> which would catch both space and tab.

Feel free to do that (one nitpick: is [^^] more readable than [^\^]?).

Should I resend the patch?

-- 
Jakub Narebski