On Fri, Sep 30, 2011 at 8:12 PM, Dave Reisner <[email protected]> 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. >
Well I called it config_parser. I meant for it to *read* files only. For readers reading from stdin is more natural. I see both functions as a variant of `read'... We can pipe around more easily, without the need of process substitution. And we don't have a convention yet. After all, getters and setters are two differnt things, I don't think we have to unify the syntax. > 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. > > # 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. > > d > Again I didn't intend to implement mutators. But if I did, I would name them: # accessors conf_key_val # print the value of key conf_key_set # Is key set? # mutators conf_set_key # set a key to either "on" or =value [1] conf_unset_key # simply comment the line(s) setting key [1] conf_set_key would add the =value part only if value is specified: conf_set_key conf key # set key on conf_set_key conf key value # set key to value I think it would be pretty clear _by comparison_ that: - The 'set' in conf_key_set is a past participle. => accessor - The 'set' in conf_set_key is a verb. => mutator It would be hard to mix those two up if we implemented them both. At last, I just wanted to make myself clear. I'm open to name changes :) >> Signed-off-by: lolilolicon <[email protected]> >> --- >> scripts/library/README | 3 ++ >> scripts/library/config_parser.sh | 56 >> ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 59 insertions(+), 0 deletions(-) >> create mode 100644 scripts/library/config_parser.sh >> >> diff --git a/scripts/library/README b/scripts/library/README >> index 1e9c962..4fb70dd 100644 >> --- a/scripts/library/README >> +++ b/scripts/library/README >> @@ -1,6 +1,9 @@ >> This folder contains code snippets that can be reused by multiple >> scripts. A brief description of each file follows. >> >> +config_parser.sh: >> +Parses simple config files such as pacman.conf. >> + >> output_format.sh: >> Provides basic output formatting functions with levels 'msg', 'msg2', >> 'warning' and 'error'. The 'msg' amd 'msg2' functions print to stdout >> diff --git a/scripts/library/config_parser.sh >> b/scripts/library/config_parser.sh >> new file mode 100644 >> index 0000000..1d125eb >> --- /dev/null >> +++ b/scripts/library/config_parser.sh >> @@ -0,0 +1,56 @@ >> +shopt -s extglob >> + >> +# Parse value of a non-repeating key from a config file. >> +# >> +# stdin: config file >> +# $1: key - shall not contain '=', or start/end with whitespaces >> +# $2: default value if not defined in config file [optional] >> +# return: >> +# 0 - print value of key >> +# 1 - print nothing >> +conf_key_val() { >> + local key val >> + while IFS='=' read -r key val; do >> + # strip whitespaces >> + key=${key##*([[:space:]])} >> + key=${key%%*([[:space:]])} >> + val=${val##*([[:space:]])} >> + val=${val%%*([[:space:]])} > > read is the opportune thing to use here -- it's faster (which surprises > me) and more concise. > > read -r key <<< "$key" > read -r val <<< "$val" Nice tip! I like it, and it also does away the need of extglob...
