GNOME Bugzilla – Bug 622598
Complete quoting for parameters in "configure.in"
Last modified: 2010-09-11 23:20:45 UTC
I recommend to be more explicit where macro expansion will not be required. http://autogen.sourceforge.net/acquoting.html http://www.gnu.org/software/autoconf/manual/m4/Quoting-Arguments.html blob: cdd6d9580966579f64d1faa525331c19bfbb18c6 http://git.gnome.org/browse/gparted/tree/configure.in
Created attachment 164513 [details] [review] update suggestion How do you think about any additional clean-ups for your Autoconf script(s)?
Thank you Markus for your suggestions and patch. I am far from being an expert in autoconf. Is the reason for placing the square brackets around text just to make it more clear that the text is not be be expanded? One tip I picked up from the first link is: As a consequence, you, the Autoconf macro developer should, in general, do everything you can to write code that either does not contain square brackets, or only contains them in balanced pairs. Based on this tip, I think that the existing configure.in file is in alignment with this tip. Another tip I picked up from the second link is: It is common practice to quote all arguments to macros, unless you are sure you want the arguments expanded. I believe that it is this advice that your patch addresses. From reviewing your patch, I have a question about the last few lines: -AM_CONDITIONAL(DISABLE_DOC, test ${enable_doc} = no) +AM_CONDITIONAL([DISABLE_DOC], [test ${enable_doc} = no]) In these lines, why would the text "test ${enable_doc} = no" be placed between square brackets? I thought that we want at least the "${enable_doc}" text to be expanded? Please note again that I do not deep knowledge about autoconf. Your advice is appreciated.
(In reply to comment #2) > I thought that we want at least the "${enable_doc}" text to be expanded? Yes - We want the variable evaluation in the generated shell script. Please distinguish this desired effect from values that will be passed to available functions in the context of the M4 programming language. Are you going to add some square brackets at the suggested places?
Yes I do plan to add these brackets. Would you be able to compare your patch with the latest code in the git repository and make any needed updates? After that I can review and apply the patch.
(In reply to comment #4) I guess that another comparison will not be needed from me because the command "git apply/am" will show if my update suggestion is still applicable for your current configuration file(s).
Thank you again Markus for this enhancement patch. I have committed your patch to the git repository for inclusion in the next release of GParted (0.6.3). The relevant git commit can be viewed at the following link: http://git.gnome.org/browse/gparted/commit/?id=c1e72d04ca36b9e7a8b4cc401d22413881738ee5 Closing this bug report.