Re: char-ranges.patch

[Akim Demaille]

| Hello, Akim!
| > Index: ChangeLog
| > from  Akim Demaille  <address@hidden>
| >     Start avoiding dependence upon character ranges.
| > 
| >     * acgeneral.m4 (_AC_INIT_DEFAULTS): Introduce `ac_cr_AZ',
| >     `ac_cr_az', `ac_cr_09', `ac_cr_alnum' and `ac_hostname'.
| >     Spread their use.
| 
| 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.


| 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?


| Akim, you should be allowed to add newlines without asking in the list :-)

:)

| >  # This variable seems obsolete.  It should probably be removed, and
| >  # only ac_max_sed_lines should be used.
| >  : ${ac_max_here_lines=38}
| > +
| > +# 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.


| > +# Avoid depending upon Character Ranges.
| 
| Why capitalized?

To highlight why variables are named ac_cr_.


| >  # Sed expression to map a string onto a valid sh and CPP variable names.
| > -ac_tr_sh='sed y%*+%pp%;s%[[^a-zA-Z0-9_]]%_%g'
| > -ac_tr_cpp='sed 
y%*abcdefghijklmnopqrstuvwxyz%PABCDEFGHIJKLMNOPQRSTUVWXYZ%;s%[[^A-Z0-9_]]%_%g'
| > +ac_tr_sh="sed y%*+%pp%;s%[[^_$ac_cr_alnum]]%_%g"
| > +ac_tr_cpp="sed y%*$ac_cr_az%P$ac_cr_AZ%;s%[[^_$ac_cr_alnum]]%_%g"
| 
| 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...


| > -      # `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???


| >        # `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 :)


| > -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.


| > -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.


| 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...

Akim