Re: [PATCH 2/3] gitweb: Add a feature for adding more branch refs
- From
Krzesimir Nowak <krzesimir@endocode.com>
- Date
- Dec 4, 2013, 12:49 UTC
- Message-ID
- <1386161398.2173.9.camel@localhost.localdomain>
- In-Reply-To
- <CANQwDwcpkbbfND9MmB9wGcL7mrhV_Mxb1amvGPgWeR344_fcfw@mail.gmail.com>
On Tue, 2013-12-03 at 21:38 +0100, Jakub Narębski wrote:
Show 28 quoted lines
> On Tue, Dec 3, 2013 at 9:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > 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). Shouldn't this
> > part do the same?
>
> Right. feature_snapshot() has here
>
> my (@fmts) = @_;
> my ($val) = git_get_project_config('snapshot');
>
> ...though git_get_project_config returns scalar.So what's the point of it? 'my @val = git_get_project_config ()' just creates an array with one element.
Show 13 quoted lines
> > > * 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)? > > This would require changes in git_get_project_config(), which would > need to be able to deal with multi-valued result (it caches these > results, so we pay only one cost of `git config` call).
Hm, actually not at all. Now, if I have a setup like Junio wrote the git_get_project_config just returns an array ref. So modifying the feature_extra_branch_refs to handle the returned value as either simple scalar or array reference should be enough.
Show 11 quoted lines
> > > * 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). > > Errr... actually git_get_project_config() strips '_' from $key, though > not for some strange reason '-'. > > BTW. though it is 'showsizes' in code, it usually is 'showSizes' in > config file (camelCase convention, lowercased by git-config).
Oi, that was an omission from my side - at first I had that git config setting with underscores. I removed them when I noticed that underscores are not used there. Apparently I missed that one.
Show 10 quoted lines
>
> >> + if ($values) {
> >> + @branch_refs = split /\s+/, $values;
> >> + }
> >> +
> >> + return @branch_refs;
> >> +}
>
>
> -- Krzesimir Nowak Software Developer Endocode AG krzesimir@endocode.com ------ Endocode AG, Johannisstraße 20, 10117 Berlin info@endocode.com | www.endocode.com Vorstandsvorsitzender: Mirko Boehm Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker Aufsichtsratsvorsitzende: Jennifer Beecher Registergericht: Amtsgericht Charlottenburg - HRB 150748 B