[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: char-ranges.patch
From: |
Pavel Roskin |
Subject: |
Re: char-ranges.patch |
Date: |
Tue, 24 Oct 2000 11:04:59 -0400 (EDT) |
Hello, Akim!
> | ac_cr_09 sounds weird. ac_cr_num would be better.
>
> I tried to keep names which could reflect both the categories such as
> :alnum:, and ranges. So maybe instead of _09 it should be _digit and
> instead of _az, it should be _lower. But, otoh, _az and _09 are more
> precisely what we mean than _lower and _digit. Hence the name. And
> also, it is shorter. Yet in different places I've had to break lines.
It was just a hint. No problem if you are strong in your preferences.
> | ac_hostname could belong to a separate patch.
>
> Yes, it could. I agree a perfect patch should separate them, but does
> it really matter here?
No. It's just the matter of convenience.
> | Akim, you should be allowed to add newlines without asking in the list :-)
>
> :)
I really mean it :-)
> | > +# Name of the host.
> | > +# hostname on some systems (SVR3.2, Linux) returns a bogus exit status,
> | > +# so uname gets run too. Use two double quotes for font-lock.
> | > +ac_hostname=`(hostname || uname -n) 2>/dev/null | sed 1q`
> |
> | Double quotes??? Font-lock???
> | This part of the comment should probably be left where it stood.
>
> Heck. Thanks, sorry about that.
Aha, it was another hint, but this time it turned out to be more useful.
> | > +# Avoid depending upon Character Ranges.
> |
> | Why capitalized?
>
> To highlight why variables are named ac_cr_.
Ok.
> | You replaces single quotes with double quotes. Maybe it's safe, but I would
> | feel safer if single quotes were used everywhere except variables:
> |
> | ac_tr_sh='sed y%*+%pp%;s%[[^_'$ac_cr_alnum']]%_%g'
>
> I think this is cluttering readability. I agree there are places
> where the approach you advocate is right, but I tend to think it's
> overkill here. Now, if everybody insists...
It's sufficient that you realize the danger and scan the line for things
that can possibly behave differently inside double quotes. I don't insist
on anything.
> | > - # `set' does not quote correctly, so add quotes (double-quote
> substitution
> | > - # turns \\\\ into \\, and sed turns \\ into \).
> | > + # `set' does not quote correctly, so add quotes (double-quote
> | > + # substitution turns \\\\ into \\, and sed turns \\ into \).
> |
> | That's great but please do it separately.
>
> Really!?!?! Why???
Because you would not have to ask me why :-)
> | > # `set' quotes correctly as required by POSIX, so do not add
> quotes.
> | > - sed -n '[s/^\([a-zA-Z0-9_]*_cv_[a-zA-Z0-9_]*\)=\(.*\)/\1=\2/p]'
> | > + sed -n \
> | > +
> ["s/^\\([_$ac_cr_alnum]*_cv_[_$ac_cr_alnum]*\\)=\\(.*\\)/\\1=\\2/p"]
> |
> | Again single quoting vs. double quoting.
>
> Yep, so what :)
Just a warning sign.
> | > -pushdef(AC_Prog, translit($1, a-z, A-Z))dnl
> | > +pushdef([AC_Prog], translit([$1], [a-z], [A-Z]))dnl
> |
> | It's a separate issue again. It's fine with me if you apply such changes
> without
> | asking anybody.
>
> Yes, OK, OK, I go ahead and apply this part of the patch now.
Great! Thank you!
> | > -AC_PATH_PROG(AC_Prog, $1)
> | > +AC_PATH_PROG(AC_Prog, [$1])
> |
> | Why not [AC_Prog] in this case?
>
> Because I'm was not sure AC_PATH_PROG quotes properly, so it's just
> for safety. But you are right, I should have checked and quoted.
Thank you!
> | What has all this to do with character ranges?
>
> Just the fact that this was on my way and I hate wasting time in
> making zillions of mini patches :( Bad attitude I guess, but...
No, not bad attitude! Akim, you are doing a great job! Autoconf wouldn't
ever be like it is without you!
Probably you should just find better tools. I have absolutely no problems
making many patches.
--
Regards,
Pavel Roskin
- char-ranges.patch, Akim Demaille, 2000/10/23
- Re: char-ranges.patch, Lars J. Aas, 2000/10/24
- Re: char-ranges.patch, Akim Demaille, 2000/10/24
- Re: char-ranges.patch, Lars J. Aas, 2000/10/24
- Re: char-ranges.patch, Akim Demaille, 2000/10/24
- Re: char-ranges.patch, Lars J. Aas, 2000/10/24
- Re: char-ranges.patch, Akim Demaille, 2000/10/24
- Re: char-ranges.patch, Alexandre Oliva, 2000/10/24
- Re: char-ranges.patch, Akim Demaille, 2000/10/24
- Re: char-ranges.patch, Alexandre Oliva, 2000/10/24