Hi!
On Thu, Jul 03, 2008 at 06:24:53PM +0200, Jakub Narebski wrote:
> There are also a few things which I'd like some comments about:
>
> * Do config_val_to_bool and config_val_to_int should be exported
> by default?
Yes with the current API, not with object API (see below). But if
exported by default, there should be definitely a git_ prefix.
> * Should config_val_to_bool and config_val_to_int throw error or
> just return 'undef' on invalid values? One can check if variable
> is defined using "exists($config_hash{'varname'})".
I think that it's more reasonable to throw an error here (as long as
you don't throw an error on undef argument). This particular case is
clearly a misconfiguration by the user and you rarely need to handle
this more gracefully, I believe.
> * How config_val_to_bool etc. should be named? Perhaps just
> config_to_bool, like in gitweb?
What about Git::Config::bool()? ;-) (See below.)
> * Is "return wantarray ? %config : \%config;" DWIM-mery good style?
> I am _not_ a Perl hacker...
I maybe wouldn't be as liberal myself, but I can't tell if it's a bad
style either.
> * Should ->get_config() use ->command_output_pipe, or simpler
> ->command() method, reading whole config into array?
You have the code written already, I'd stick with it.
> * 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.
> * Should we perltie hash?
I agree with Lea that we should actually bless it. :-) Returning a
Git::Config object provides a more flexible interface, and you can also
do $repo->get_config()->bool('key') which is quite more elegant way than
the val_ functions, I think. 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.
> As this is an RFC I have not checked if manpage (generated from
> embedded POD documentation) renders correctly.
By the way, unless you know already, you can do that trivially by
issuing:
perldoc perl/Git.pm
--
Petr "Pasky" Baudis
The last good thing written in C++ was the Pachelbel Canon. -- J. Olson
--
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
