Re: [PATCH] glib configure.in patches

[Art Haas <ahaas neosoft com>]

On Sun, Jul 07, 2002 at 03:38:21PM -0400, Owen Taylor wrote:
> 
> Art Haas <ahaas neosoft com> writes:
> 
> > This patch is against the glib-2-branch, not against the
> > head. I don't know whether or not the patch applies directly
> > to the head, as I'm sticking with the branch for now. I
> > also have similar patches for atk, pango, and gtk+ which
> > I'll be glad to forward along if there's interest. 
> 
> I'm not going to apply any changes like this to the glib-2-0
> branch. I don't really have time to merge it to HEAD myself.
> (My guess is that it will 90% apply cleanly and 10% not.)
> So I need to someone to merge it if it is going to get applied.
> 

I'll look at moving off the branch and see about regenerating
a patch for the HEAD. The glib-2-0 branch has seemed to stablize
lately, as have the branches for atk/pango/gtk.

> 
> >  dnl declare --enable-* args and collect ac_help strings
> > -AC_ARG_ENABLE(debug, [  --enable-debug=[no/minimum/yes] turn on debugging [default=$debug_default]],,enable_debug=$debug_default)
> 
> > +AC_ARG_ENABLE([debug],
> > +              AC_HELP_STRING([--enable-debug=@<:@no/minimum/yes@:>@],
> > +	                     [turn on debugging @<:@default=$debug_default@:>@]),
> > +	      [],
> > +	      [enable_debug=$debug_default])
> 
> Ugh, these 'quadriglyphs' are incredibly unreadable. I assume the
> problem is that AC_HELP_STRING does multiple levels of expansion on
> it's arguments?

I guess so :-/ ...

I used them so that the output of the "./configure --help"
would still present default values with "[" and "]" ...

   --enable-foo             activates foo [default=on]

If a slight change wasn't unacceptable, the default values
could come out in say "<" and ">" characters ...

   --enable-foo             activates foo <default=on>

... and the quadrigraphs could be removed. As I've changed many
configure.(in|ac) files, I don't find them that difficult to read,
but they are off-putting at first glance.

> > @@ -237,8 +260,8 @@
> >    fi
> >  fi
> >  
> > -AC_CYGWIN
> > -AC_EXEEXT
> > +dnl AC_CYGWIN
> > +dnl AC_EXEEXT
> 
> Commenting out macros does nothing but create confusion.
> If they aren't needed any more, they need to be removed.
>   

Agree. My fault for not removing them before making the patch
to send.

> > @@ -751,7 +778,9 @@
> >  if test "$gtk_ok" = "yes"; then
> >      AC_MSG_RESULT([yes, found in sys/types.h])
> >  else
> > -    AC_EGREP_HEADER(fd_set, sys/select.h, gtk_ok=yes)
> > +    AC_EGREP_HEADER([fd_set],
> > +                    [sys/select.h],
> > +                    [gtk_ok=yes])
> 
> Why this change? It's really important to keep patches as small
> as possible, and as much "only one thing per patch", or they
> are a lot harder to review. There are a bunch of places
> where you've added extra qutoting for  no apparent reason.
> 

I added lots of explicit quotes and did reformatting like you
listed above because that's just how I like to see things, and
to me make the macros easier to follow. Autoconf 2.5X is much
more demanding w.r.t argument quoting than the 2.13 version,
so I'd rather have a few extra quotes in the configure.in file
than find out later that they are missing.

Thanks for all the work with glib/atk/pango/gtk/gnome/...!

Art Haas

-- 
They that can give up essential liberty to obtain a little temporary
safety deserve neither liberty nor safety.
 -- Benjamin Franklin, Historical Review of Pennsylvania, 1759