On Tue, 2013-12-03 at 21:38 +0100, Jakub Narębski wrote:
> On Tue, Dec 3, 2013 at 9:15 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Krzesimir Nowak <krzesimir@xxxxxxxxxxxx> 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.
>
> > * 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.
>
> > * 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.
>
> >> + if ($values) {
> >> + @branch_refs = split /\s+/, $values;
> >> + }
> >> +
> >> + return @branch_refs;
> >> +}
>
>
>
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesimir@xxxxxxxxxxxx
------
Endocode AG, Johannisstraße 20, 10117 Berlin
info@xxxxxxxxxxxx | 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html