Re: [PATCH 3/3] configure: Get help text from meson_options.txt

[Thomas Huth]

On 30/08/2021 17.30, Eric Blake wrote:
> On Sun, Aug 29, 2021 at 07:32:10PM +0200, Thomas Huth wrote:
> > It's cumbersome to maintain the option help texts twice, once in the
> > "configure" script and once in meson_options.txt. So let's add some logic to
> > the configure script to read most of the help texts from meson_options.txt.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   configure | 89 ++++++++++++++++---------------------------------------
> >   1 file changed, 25 insertions(+), 64 deletions(-)
> >
> > diff --git a/configure b/configure
> > index cb125c3f84..8446b7b3ea 100755
> > --- a/configure
> > +++ b/configure
>
> > +
> > +current_feature=""
> > +current_desc=""
>
> current_desc is unused below.
>
> > +while read word1 word2 ; do
>
> A bit misleading in the variable names.  As a sample, you are parsing:
>
> option('malloc_trim', type : 'feature', value : 'auto',
>         description: 'enable libc malloc_trim() for memory optimization')
>
> which read then splits into:
>
> word1="option('malloc_trim'," word2="type : 'feature', value : 'auto',"
> or
> word1="description:" word2="'enable libc malloc_trim() for memory 
> optimization')"
>
> Better might be the names $first and $rest.  You could also play with
> $IFS for more precise splitting, but your hack is good enough for the
> current usage.
>
> > +   case "$word1" in
> > +   option*)
> > +       if echo "$word2" | grep -q "type[ ]*: 'feature'"; then
>
> Unlike my complaint in patch 1 about using echo on user-supplied text,
> here you are using it on test in meson_options.txt which is presumably
> not going to contain \ or leading -.
>
> > +           current_feature=$(echo $word1 | sed -e "s/option('//" -e 
> > "s/',.*$//")
> > +       else
> > +           current_feature=""
> > +       fi
>
> If desired, you can save some fork()ing by doing:
>
> case "$word2" in *type*:*"'feature'")
>      current_feature=${word1%\'*}
>      current_feature=${current_feature#*\'}
>    *) current_feature=""
> esac
>
> > +   ;;
> > +   description:)
> > +       if [ -n "$current_feature" ]; then
> > +           printf "  %-15s %s\n" "$current_feature" \
> > +                  "$(echo "$word2" | sed -e "s/^'//" -e "s/'.*$//")"
>
> Similarly,
>
> if [ "$current_feature" ]; then
>    word2=${word2%\'*}
>    printf "  %-15s %s\n" "$current_feature" "${word2#\'}"
> fi
>
> > +           current_feature=""
> > +       fi
> > +   ;;
> > +   esac
>
> Missing a *) catchall case (probably a good idea to reset
> current_feature back to "" on lines that don't match the two forms you
> care about).

Thanks for your review, Eric! Your shell script knowledge is always amazing! :-)

I'll wait for Paolo first to chime in to see whether he rather wants to 
resume his variant with the Perl or Python script, but if not, I'll include 
your suggestions in the next version of the patch.

 Thomas