Re: awk for config headers

[Ralf Wildenhues]

Hello Paul,

Thanks for the review and updated patch!

* Paul Eggert wrote on Wed, Oct 03, 2007 at 01:43:01AM CEST:
> Ralf Wildenhues <address@hidden> writes:
> 
> >> The 'split' doesn't need to be done unless the line starts with '#'.
> >
> > Why is that a problem?  The above is short to write, and I don't see how
> > I can avoid regex evaluation at all on nonmatching lines.
> 
> It's mostly a style issue, I think it's clearer to express the
> computation as something like "if the line is an interesting one, then
> change it" rather than "do some computation on the line, then test
> whether it's interesting, and change it if it's interesting".

Agreed.

> > Hmm.  I would like to extract white space before `#', and between `#'
> > and `define' or `undef'.  Is this easier to read?
> >
> >       split(line, ws1, "#")
> >       split(ws1[2], ws2, substr(defundef, 1, 1))
> >       line = ws1[1] "#" ws2[1] "define " macro " " value
> 
> Better, but I think it's even clearer to use "index".

Hmm, Solaris awk has `index', but Autoconf's manual says:
|      Traditional Awk supports only the predefined functions `exp',
|      `int', `length', `log', `split', `sprintf', `sqrt', and `substr'.

Which is why I tried to avoid it.  ;-)

> Here's a
> proposed revision of the patch, which I've tested only on Debian
> stable.  I tweaked it to simplify it a bit more (avoid the D_is_set
> business, for example, by making sure the entry cannot be the empty
> string).  I ran it once; it's a tad slower (0.06% slower for "make
> check" on Debian stable) but I don't know whether the speed difference
> is in my machine's noise.

I don't care about the speed difference in an operation that will not
dominate for any reasonable package.  Your approach is much nicer.

FWIW, I checked that, since for the first piece of the split-up literal
string we overestimate its length anyway, there is no problem here.

Also, your patch seems to work on the systems I tested on.  (I had
a failure on IRIX, but I think that was due to an independent patch
lurking in my tree.)

If you can assert that `index' is safe to use, I'll apply the patch.

Cheers,
Ralf