[pacman-dev] [PATCH 1/1] scripts/library: add config_parser.sh

[Dan McGee]

On Fri, Sep 30, 2011 at 7:12 AM, Dave Reisner <d at falconindy.com> wrote:
> On Fri, Sep 30, 2011 at 07:37:48PM +0800, lolilolicon wrote:
>> config_parser.sh provides functions to support parsing simple config
>> files, pacman.conf in particular.  Our scripts should never use a
>> simple eval to get config values such as DBPath.
>>
>> conf_key_val() is modified from pacman-key's get_from() function. Please
>> note conf_key_val() and conf_key_set() both read config file from stdin,
>> instead of $1.
>
> I don't see the advantage of doing it this way. If/when we add a
> function like conf_keyval_set() (which actually updates a file) we'll
> have to ignore this convention and pass the filename, for obvious
> reasons.
Agreed- reading from stdin is a bit overkill for these purposes.

> At the risk of painting my shed green, I dislike your naming convention,
> particularly conf_key_set, which doesn't set anything at all. I would
> have thought that something such as the following would be appropriate:
>
> # accessors
> conf_key_isset() # determines existance of a key w/o an associated value.
> conf_keyval_get() # gets the value of a key.
These are pacman.conf only, right? Why not
    pacman_conf_get()
    pacman_conf_isset()

Another set of issues that may/may not be covered, but I didn't see
anything at first glance:
1) is this [options] only, or are sections not even handled? Big
problem if this is to be made universal, or one is looking up
SigLevel... I think one needs to call this with both a file path and a
section name. We probably also need a function that returns all the
section names (pacman_conf_get_sections?).
2) How are multivalued options handled? Remembering the following are
equivalent:
    HoldPkg = foo1 foo2
    HoldPkg = foo3
and
    HoldPkg = foo1 foo2 foo3

> # mutators
> conf_key_set() # sets a key without a value, as long as it doesn't exist.
> conf_keyval_set() # sets a key with a value, as long as it doesn't exist.
>
> And of course, accessors return 0/1 to indicate existance, mutators
> return 0/1 to indicate write success/failure.
If we start mutating the config file, I'll be a bit scared- I don't
see a whole lot of need for these anytime soon.

-Dan