Re: [RFC/PATCH (WIP)] Git.pm: Add get_config() method and related subroutines

From: Jakub Narebski <jnareb@gmail.com>
Date: Jul 12, 2008, 14:31 UTC
Message-ID: <200807121631.51406.jnareb@gmail.com>
In-Reply-To: <20080712134553.GC10151@machine.or.cz>

On Sat, Jul 12, 2008, Petr Baudis wrote:

> On Sat, Jul 12, 2008 at 02:35:48PM +0200, Jakub Narebski wrote:
>> On Sat, Jul 12, 2008, Petr Baudis wrote:

Show 18 quoted lines

>> On the other hand if one uses (assuming now Git::Config object API)
>> 
>>   $conf = $repo->get_config('section');
>> 
>> one could access configuration variable values _without_ prefixing
>> them with subsection name, i.e.
>> 
>>   $conf->get('variable');
>> 
>> and not
>> 
>>   $conf->get('section.variable');
>> 
>> 
>> I'm not sure if it is much of an improvement.
> 
> I would propose not to go with this feature yet and add it only when
> justified by accompanied cleanups of real code using this.

O.K.

Show 26 quoted lines

>> The former plays better with
>> 
>>   $r->get_config()->bool('gitweb.blame');
>> 
>> but this is nevertheless not recommended workflow; you can use
>> 
>>   $r->config_bool('gitweb.blame');
>> 
>> and it would be faster (unless some Perl/OO trickery with singletons
>> and the like).
>> 
>>  Recommended workflow (code) is
>> 
>>   $c = $r->get_config();
>>   ...
>>   $c->get_bool('gitweb.blame');
>> 
>> or something like that.
> 
> I thought that the get_config() method would be reusing existing
> Git::Config instances no-matter-what? (Otherwise, you can get into
> rather tricky issues like one part of the code modifying the config and
> the other not noticing the change, etc. And I can se no benefit. If you
> want to reload the config, we might have method like reload(), but I
> don't think that would be very useful except perhaps in some kind of
> mod_perl'd gitweb scenario.)

Somehow I forgot that $repo can remember existing Git::Config, generate it only once (and remember it) on demand but fully, and then serve it. It probably is some OO "pattern" like SingletonFactory or sth...

It would probably play better with Git::Repo object interface, but can I think work as well with Git->repository(...) instance.

As to changing config: I wasn't even thinking about that... and I don't think that any command written in Perl which uses or can use either Git or Git::Repo API needs writing config files.

> But we're arguing about nothing, really, except the exact amount of
> coolness of the Git::Config approach. :-)

True. For me the fact that one can write $c->value('diff.renameLimit') or $c->value('diff.renamelimit'); is teh winner ;-)

-- 
Jakub Narebski
Poland