Lea Wiemann wrote:
> Jakub Narebski wrote:
>>
>> Add get_config([PREFIX]) method [...]
>
> FWIW, I don't think it'll make much of a difference for gitweb, since
> the 'git config -l' output is cached anyway, but it's good someone's
> extracting this. Do you have any user for that function besides gitweb?
If I remember correctly git-cvsserver used to have Perl parser for
_simplified_ config format, but now it uses `git config -l -z` or
`git config -l`. Other git commands written in Perl which uses a lot
of configuration option could use it, like git-svn or git-send-email,
although for them shaving a little bit of execution time is not that
important (git-svn from what I understand still calls git-config
for each config variable).
>> * Should config_val_to_bool and config_val_to_int throw error or
>> just return 'undef' on invalid values?
>
> I suspect that if you have, say, command line tools, throwing an error
> is better UI behavior than silently ignoring the entry. And developers
> can always catch errors if they want to.
Actually for ->config_bool(<VARIABLE>) throwing error was the _only
way_ to distinguish between *non-existent* config variable (which we
can test using exists($config{<VARIABLE>}) when using ->get_config(),
and for which ->config_bool(<VARIABLE>) returned 'undef'), and
*non-bolean* value (for which config_val_to_bool($config{<VARIABLE>})
can return 'undef', and for which ->config_bool(<VARIABLE>) threw
error).
So we don't _need_ to _throw_ an error; we can detect error condition
in other way.
>> * Is "return wantarray ? %config : \%config;" DWIM-mery good style?
>
> Gitweb uses it as well, and it seems reasonable IMVHO.
Errr... if I remember correctly, the code in gitweb is by yours truly
:-), and as I have stated I am *not* a Perl hacker.
>> * Should ->get_config() use ->command_output_pipe, or simpler
>> ->command() method, reading whole config into array?
>
> Does it make a difference? If you're worried about performance, config
> files are so short that it won't matter; use the easier path.
I think using ->command() would be easier...
>> * What should ->get_config() method be named? ->get_config()
>> or perhaps ->config_hash(), or ->config_hashref()?
>
> Regarding the method naming, how about making this an object oriented
> interface? [...] if you can wait a week or so, you could maybe
> integrate this into the Git::Repo interface [...]
I'd rather have both functional (of sorts) and object interface.
Git::Repo / Git::Config could use methods / subroutines from Git.pm;
>> * What should ->get_config() have as an optional parameter:
>> PREFIX (/^$prefix/o), or simply SECTION (/^(?:$section)\./o)?
>
> Off the top of my head, I don't see much need for a prefix parameter, so
> I'd go for 'section'.
O.K. (that is by the way how it is done in gitweb).
> I haven't been able to answer all of the questions, but I hope this helps.
Thanks a lot!
--
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
