+ [AS_CASE(["$enableval"], no quotes
+ [[[0-5]]], warning_level="$enableval", bracket the second argument Approved with those two changes and your own testing, of course. On Tue, Aug 18, 2009 at 1:41 AM, JonY<[email protected]> wrote: > On 8/18/2009 00:28, JonY wrote: >> >> On 8/17/2009 22:37, NightStrike wrote: >>> >>> 1) Put the "AC_MSG_CHECKING" at the beginning, before you start the >>> checking. >> >> Ok, done. >> >>> 2) Features like this should be an "enable" thing, not a "with" thing. >>> "with" things are more for external tools. >> >> Ok, done. I was wondering about that... >> >>> 3) Keep the same formatting that's elsewhere in the file, or otherwise >>> change the whole file. For instance, look at the formatting and >>> indentation of other HELP_STRING instances. >> >> I hope the new indentation is better. >> >>> 4) You mention CPPFLAGS in your comment, but it's not used for this >>> kind of stuff >> >> I was using it, but forgot to change it. Sorry, outdated comment. >> >>> 5) You deal with CXXFLAGS -- Where do we use C++? We may definitely >>> do it.. I don't remember >> >> The variable is mainly used to allow C++ specific warning flags, not all >> of the warning parameters can be shared. The C++ code is in the testcase >> directory. >> >>> 6) This line in Makefile.am is probably wrong: >>> +am_cxxfla...@add_cxx_only_warning_flags@ @ADD_CXX_ONLY_WARNING_FLAGS@ >>> >>> One of those should probably be ADD_C_CXX... >>> >> >> Thanks for catching that. >> >>> 7) This line should really be a part of the "action if given" / >>> "action if not given" section of the option string handler, to be >>> consistent with the rest of the file: >>> >>> +AS_IF([test "$warning_level" == yes], [warning_level=3], [test >>> "$warning_level" == no], [warning_level=0]) >>> >>> For instance, instead of "+ [warning_level=$withval],", >>> just operate on withval directly: >>> >>> [AS_IF([test "$withval" == yes], >>> [warning_level=3],[warning_level=$withval])], >>> >>> Same for the action if not given, and withval = no. >>> >>> Also, as before, follow the formatting of the rest of the file >>> regarding indentation. >>> >> >> Ok, the logic check is now included in the AC_ARG_ENABLE part. >> >>> On a more general note, I'm wondering if there isn't a better way to >>> compute all of this in Makefile.am. Automake supports conditionals, >>> which makes the additive nature of successive levels much more simply >>> handled there. The idea is that you should keep configure stuff in >>> configure, and make stuff in make. >>> >>> >> >> With AM_CONDITIONALs, but the code below is the best I can come up with. >> >> if WARNLEVEL0 >> C_Warn0=... >> C_CXX_WARN0=... >> endif >> if WARNLEVEL1 >> C_Warn0=... >> C_CXX_WARN0=... >> endif >> AM_CFLAGS=$(C_Warn0) $(C_Warn1) $(C_Warn2)... >> >> If its ok, I'll use the above code in Makefile.am >> >> Updated patch attached. Suggestions welcome, especially simplifying the >> AM_CFLAGS part. Testers are welcome too. > > Hi, > Attached updated patch, this time using case, much cleaner than if/else > tests. The "[yes], [warning_level=3]," part is probably redundant. > > ------------------------------------------------------------------------------ > Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day > trial. Simplify your report design, integration and deployment - and focus > on > what you do best, core application coding. Discover what's new with > Crystal Reports now. http://p.sf.net/sfu/bobj-july > _______________________________________________ > Mingw-w64-public mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/mingw-w64-public > > ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
