[Ovirt-devel] [PATCH] Add trivial error() function and change code to use it

[Jim Meyering]

"Jeff Schroeder" <jeffschroed at gmail.com> wrote:
...
> Speaking of that, I've got a few silly ideas that you might have a better
> way of doing. While looking at the ovirt-server-install script, there is a
> FIXME that asks for input if it can't find something like the dns srv
> records. I would love your input on this and especially on validate().
> Is there a better way to do it? The code is rife with bashisms such as
> [[ ]], ==, local varname, and the shebang is bash so a few more won't hurt.
>
> prompt() {
>     # Bourne shell's "return" doesn't support strings so use a global
>     unset answer
>     local question="$1";    shift
>     test -z "$question" && return
>     while [ -z "$answer" ]; do
>         printf '%s?: ' "$question"
>         read answer
>     done
> }

Better not to rely on a global variable like $answer.
Instead, just return the result.
That makes the calling code more readable.

A prompt function is more useful when you can provide a default value.
That makes it convenient to prompt with "Ok to rm -rf /? (y/N)" where
"no" is the default.  Otherwise, each caller has to handle any default
mapping manually.

> validate() {
>     local text=$1;       shift
>     local pattern="$1"; shift

No need for shifts here.
And the double quotes are not needed, so use this:
     local text=$1
     local pattern=$2

>     test -z "$pattern" -o -z "$text" && return 1
>     printf '%s\n' "$text" | egrep -qs "$pattern" &>/dev/null

It'd be nice to diagnose-and-die upon invalid pattern or any other error,
Similarly, it'd be better not to discard stderr.
Using "grep -E" is generally preferred to using "egrep".

Of course, all of this can be prefaced with...
if you really want to write maintainable and robust scripts,
you shouldn't be using the bourne shell ;-)