bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: gnulib-tool nits


From: Bruno Haible
Subject: Re: gnulib-tool nits
Date: Sat, 23 Jun 2007 14:53:41 +0200
User-agent: KMail/1.5.4

Ralf Wildenhues wrote:
> Another rough pass over gnulib-tool's tendency to spawn more processes
> than it needs to.

Why not. But why don't you concentrate on the loops of func_import when
doing that? gnulib-tool will not become measurably faster by optimizing
code that is executed only once.

>       (self_abspathname): Rewrite algorithm to set it, reindent.

Regression: PATH values with a trailing empty field, such as
  PATH=/usr/bin:/bin:
are not handled correctly. As you know, such empty fields are equivalent to
".".
  PATH=/usr/bin:/bin:
is thus equivalent to
  PATH=/usr/bin:/bin:.
but your code treats it like
  PATH=/usr/bin:/bin

(I guess you copied that idiom from somewhere, and it was already wrong
at the original place.)

The indentation is not right: Please keep a 2-space indentation for the
statements inside a case-block, like in the rest of gnulib-tool:
     case something in
       *)
         statement...
         ;;
     esac

>       (func_emit_lib_Makefile_am, func_emit_tests_Makefile_am)
>       (func_create_megatestdir): Merge some sed scripts.

Here too, the indentation is not right:

                     -e 's,October,10,'   -e 's,Oct,10,' \
                     -e 's,November,11,'  -e 's,Nov,11,' \
                     -e 's,December,12,'  -e 's,Dec,12,' \
                     -e 's,^,00,'         -e 's,^[0-9]*\([0-9][0-9] \),\1,' \
                     -e 's,^\([0-9]*\) \([0-9]*\) \([0-9]*\),\3\2\1,'`

You make it look like the second-to-last expression was there to handle
the 3-letter abbreviated months, and the expression before it was there to
handle the unabbreviated month names. Just confusing.

Bruno





reply via email to

[Prev in Thread] Current Thread [Next in Thread]