Hello Paul, Thanks for the review and updated patch!
* Paul Eggert wrote on Wed, Oct 03, 2007 at 01:43:01AM CEST: > Ralf Wildenhues <[EMAIL PROTECTED]> 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
