On Sat, Jul 12, 2008, Petr Baudis wrote:
> On Thu, Jul 10, 2008 at 01:33:36AM +0200, Jakub Narebski wrote:
>> On Wed, Jul 09, 2008, Petr "Pasky" Baudis wrote:
>>> On Thu, Jul 03, 2008 at 06:24:53PM +0200, Jakub Narebski wrote:
>>>> * What should ->get_config() have as an optional parameter:
>>>> PREFIX (/^$prefix/o), or simply SECTION (/^(?:$section)\./o)?
>>>
>>> Do we even _need_ a parameter like that? I don't understand what is
>>> this interface trying to address.
>>
>> For example if one wants to access _all_ variables in gitweb.* section
>> (or in gitcvs.* section), and _only_ config variables in given section.
>
> But what is the practical benefit? Does anyone use a config file long
> enough that this makes any difference?
Probably not.
> Or if you mean foreach use, it's trivial to
>
> foreach (grep { /^gitweb\./ } keys %$config)
>
> or provide a method of Git::Config that will return this list, but it
> does not seem appropriate to have specific Git::Config instances for
> this. (Besides, if the script also needs to access a single variable
> from a different section, it will need to re-parse the config again.)
>
> So I think your approach would be good only if there are multiple
> methods of Git::Config that would operate on the whole config and needed
> a way to be restricted; is that the case?
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.
>> BTW. what should non-typecasting should be named? $c->get(<VAR>),
>> $c->value(<VAR>), $c->param(<VAR>), or something yet different?
>
> I would prefer 'get' since it's the shortest and most clear, but 'value'
> would be fine too, I suppose (and more in line with bool etc.).
Well, that would have to be decided before other programs could use
this API.
We can have $c->value(<VAR>), $c->bool(<VAR>), $c->int(<VAR>) and
(in the future) $c->color(<VAR>) and $c->colorbool(<VAR>)... or we can
have $c->get(<VAR>), $c->get_bool(<VAR>) etc.
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.
>>> Also, having accessors for special types lets you return undef when
>>> the type really isn't defined, instead of e.g. true with current
>>> config_val_bool, which is clearly bogus and requires complicated code
>>> on the user side.
>>
>> I don't follow you. Didn't we agree on casting an error when variable
>> is not of given type?
>
> Sorry, s/type really/variable really/. According to your original code,
>
> config_val_bool(undef)
>
> would return true, while the undef could be from both non-existent and
> unassigned variable. (This 'unassigned variables' case is really
> annoying to handle.)
That is why you should check exists($config{<VAR>}) first.
--
Jakub Narebski
Poland
--
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
