[Top][All Lists]
[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
Re: gnulib-tool nits, Paul Eggert, 2007/06/26