Re: [PATCH v2 01/11] gitweb: introduce remote_heads feature

From: Jakub Narebski <jnareb@gmail.com>
Date: Nov 14, 2008, 18:15 UTC
Message-ID: <200811141915.17680.jnareb@gmail.com>
In-Reply-To: <1226616555-24503-2-git-send-email-giuseppe.bilotta@gmail.com>

On Thu, 13 Nov 2008, Giuseppe Bilotta wrote:

> With this feature enabled, remotes are retrieved (and displayed)
> when getting (and displaying) the heads list.

I think it would be good idea to add in commit message idea _why_ such feature would be useful, for example

  This is useful if you want to use git-instaweb to examine the state
  of repository, influding remote-tracking branches, or a repository
  is fork of other repository, and remote-tracking branches are used
  to see what commits this fork has in addition to those from forked
  (main) repository.

Or something like that.

It would be also in my opinion a good idea to modify git-instaweb.sh (I guess better in separate commit) to make it make use of this new feature... unless it does it already, doesn't it?

Show 16 quoted lines

> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>  gitweb/gitweb.perl |   31 +++++++++++++++++++++++++++++--
>  1 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 933e137..b6c4233 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -327,6 +327,18 @@ our %feature = (
>  	'ctags' => {
>  		'override' => 0,
>  		'default' => [0]},
> +
> +	# Make gitweb show remotes too in the heads list

I'm not native engish speaker, but shouldn't instead of "remotes too" be "also remotes" or "remotes also"?

Show 28 quoted lines

> +
> +	# To enable system wide have in $GITWEB_CONFIG
> +	# $feature{'remote_heads'}{'default'} = [1];
> +	# To have project specific config enable override in $GITWEB_CONFIG
> +	# $feature{'remote_heads'}{'override'} = 1;
> +	# and in project config gitweb.remote_heads = 0|1;
> +	'remote_heads' => {
> +		'sub' => \&feature_remote_heads,
> +		'override' => 0,
> +		'default' => [0]},
>  );
>  
>  sub gitweb_check_feature {
> @@ -392,6 +404,18 @@ sub feature_pickaxe {
>  	return ($_[0]);
>  }
>  
> +sub feature_remote_heads {
> +	my ($val) = git_get_project_config('remote_heads', '--bool');
> +
> +	if ($val eq 'true') {
> +		return (1);
> +	} elsif ($val eq 'false') {
> +		return (0);
> +	}
> +
> +	return ($_[0]);
> +}

Hmmm... I think it is hight time to provide option to git_get_project_config to _not_ use backward compatibility with 'git config --bool', i.e. return Perl bool, and not 'true'/'false' string.

But this is I think outside scope of this patch...

Show 26 quoted lines

> +
>  # checking HEAD file with -e is fragile if the repository was
>  # initialized long time ago (i.e. symlink HEAD) and was pack-ref'ed
>  # and then pruned.
> @@ -2642,10 +2666,12 @@ sub git_get_heads_list {
>  	my $limit = shift;
>  	my @headslist;
>  
> +	my ($remote_heads) = gitweb_check_feature('remote_heads');
> +
>  	open my $fd, '-|', git_cmd(), 'for-each-ref',
>  		($limit ? '--count='.($limit+1) : ()), '--sort=-committerdate',
>  		'--format=%(objectname) %(refname) %(subject)%00%(committer)',
> -		'refs/heads'
> +		'refs/heads', ( $remote_heads ? 'refs/remotes' : '')
>  		or return;
>  	while (my $line = <$fd>) {
>  		my %ref_item;
> @@ -2656,8 +2682,9 @@ sub git_get_heads_list {
>  		my ($committer, $epoch, $tz) =
>  			($committerinfo =~ /^(.*) ([0-9]+) (.*)$/);
>  		$ref_item{'fullname'}  = $name;
> -		$name =~ s!^refs/heads/!!;
> +		$name =~ s!^refs/(head|remote)s/!!;
>  
> +		$ref_item{'class'} = $1;

Nice catch.

Show 7 quoted lines

>  		$ref_item{'name'}  = $name;
>  		$ref_item{'id'}    = $hash;
>  		$ref_item{'title'} = $title || '(no commit message)';
> -- 
> 1.5.6.5
> 
>

-- 
Jakub Narebski
Poland