Re: [PATCH 2/3] gitweb: Add a feature for adding more branch refs

[Junio C Hamano <gitster@pobox.com>]

Krzesimir Nowak <krzesimir@endocode.com> writes:

> @@ -626,6 +640,17 @@ sub feature_avatar {
>  	return @val ? @val : @_;
>  }
>  
> +sub feature_extra_branch_refs {
> +	my (@branch_refs) = @_;
> +	my $values = git_get_project_config('extra_branch_refs');

Hmph.  Three points.

* Almost all callers of this function use

    my ($val) = git_get_project_config(...);
    my @val = git_get_project_config(...);

  to expect that the function returns a list of things (and grab the
  first one among them, not the length of the list).  Shoudln't this
  part do the same?


* Wouldn't this be a good candidate for a multi-valued configuration
  variable, e.g. shouldn't this

	[gitweb]
		extraBranchRefs = wip
		extraBranchRefs = sandbox other

  be parsed as a three-item list, qw(wip sandbox other)?
                

* I think the $key parameter to git_get_project_config() eventually
  is used to look up a key in the Git-style configuration file, and
  the 'words_with_underscore' goes against our convention (cf. see
  how 'show-sizes' feature is spelled as 'showsizes' there).

> +	if ($values) {
> +		@branch_refs = split /\s+/, $values;
> +	}
> +
> +	return @branch_refs;
> +}