git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: Petr Baudis <pasky@suse.cz>
Cc: git@vger.kernel.org, Lea Wiemann <lewiemann@gmail.com>
Subject: Re: [RFC/PATCH (WIP)] Git.pm: Add get_config() method and related subroutines
Date: Sat, 12 Jul 2008 16:31:50 +0200	[thread overview]
Message-ID: <200807121631.51406.jnareb@gmail.com> (raw)
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:
 
>> 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.
 
>> 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

  reply	other threads:[~2008-07-12 14:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-03 16:24 [RFC/PATCH (WIP)] Git.pm: Add get_config() method and related subroutines Jakub Narebski
2008-07-03 20:30 ` Lea Wiemann
2008-07-03 23:45   ` Jakub Narebski
2008-07-07 19:24     ` Lea Wiemann
2008-07-09 15:23       ` Petr Baudis
2008-07-09 16:03 ` Petr Baudis
2008-07-09 23:33   ` Jakub Narebski
2008-07-12  1:47     ` Petr Baudis
2008-07-12 12:35       ` Jakub Narebski
2008-07-12 13:45         ` Petr Baudis
2008-07-12 14:31           ` Jakub Narebski [this message]
2008-07-12 18:05             ` Petr Baudis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200807121631.51406.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=lewiemann@gmail.com \
    --cc=pasky@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).