After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 710963 - Add -fdiagnostics-color support to GNOME_COMPILE_WARNINGS
Add -fdiagnostics-color support to GNOME_COMPILE_WARNINGS
Status: RESOLVED WONTFIX
Product: gnome-common
Classification: Core
Component: general
git master
Other All
: Normal enhancement
: ---
Assigned To: Gnome Common Maintainer(s)
Gnome Common Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-10-27 12:33 UTC by Philip Withnall
Modified: 2013-12-22 11:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
compiler-flags: Replace deprecated AC_HELP_STRING with AS_HELP_STRING (2.06 KB, patch)
2013-10-27 12:34 UTC, Philip Withnall
committed Details | Review
compiler-flags: Simplify a case statement slightly (871 bytes, patch)
2013-10-27 12:34 UTC, Philip Withnall
committed Details | Review
compiler-flags: Add -fdiagnostics-color=auto to GNOME_COMPILE_WARNINGS (1.94 KB, patch)
2013-10-27 12:34 UTC, Philip Withnall
rejected Details | Review

Description Philip Withnall 2013-10-27 12:33:39 UTC
Patch coming to add -fdiagnostics-color to the GNOME_COMPILE_WARNINGS macro, as per http://tecnocode.co.uk/2013/10/25/colourful-gcc-output/.
Comment 1 Philip Withnall 2013-10-27 12:34:40 UTC
Created attachment 258213 [details] [review]
compiler-flags: Replace deprecated AC_HELP_STRING with AS_HELP_STRING
Comment 2 Philip Withnall 2013-10-27 12:34:43 UTC
Created attachment 258214 [details] [review]
compiler-flags: Simplify a case statement slightly
Comment 3 Philip Withnall 2013-10-27 12:34:46 UTC
Created attachment 258215 [details] [review]
compiler-flags: Add -fdiagnostics-color=auto to GNOME_COMPILE_WARNINGS

As it doesn’t affect the type of warnings or errors reported by the compiler, it is
unconditionally enabled (subject to compiler support) for all user-supplied values
of the --enable-compile-warnings flag.
Comment 4 David King 2013-10-28 23:18:10 UTC
Comment on attachment 258213 [details] [review]
compiler-flags: Replace deprecated AC_HELP_STRING with AS_HELP_STRING

OK, please push.
Comment 5 David King 2013-10-28 23:19:15 UTC
Comment on attachment 258214 [details] [review]
compiler-flags: Simplify a case statement slightly

Looks fine, please push.
Comment 6 David King 2013-10-28 23:24:20 UTC
Comment on attachment 258215 [details] [review]
compiler-flags: Add -fdiagnostics-color=auto to GNOME_COMPILE_WARNINGS

Thanks for the patch. Sorry to say it, but I do not think that GNOME_COMPILE_WARNINGS is the place for cosmetic additions. I have been subjected to the colourful warnings on Fedora 20 and find them garish at best.
There is at least one other gnome-common bugs about cosmetic changes, such as enabling Automake silent rules in bug 580062, so it might make sense to have a macro for applying some nice style options, but I think it would have to be opt-in, and distinct from compiler warnings.
Comment 7 Philip Withnall 2013-10-29 10:45:02 UTC
Comment on attachment 258213 [details] [review]
compiler-flags: Replace deprecated AC_HELP_STRING with AS_HELP_STRING

commit 62f7546a039296e42b90ca9f9590caf9db5bc6ef
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Sun Oct 27 12:08:38 2013 +0000

    compiler-flags: Simplify a case statement slightly

 macros2/gnome-compiler-flags.m4 | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

commit eb0a7dce6b639e354b4b9736ed879921595f12fb
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Sun Oct 27 12:02:55 2013 +0000

    compiler-flags: Replace deprecated AC_HELP_STRING with AS_HELP_STRING

 macros2/gnome-compiler-flags.m4 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Comment 8 Philip Withnall 2013-10-29 10:50:50 UTC
(In reply to comment #6)
> (From update of attachment 258215 [details] [review])
> Thanks for the patch. Sorry to say it, but I do not think that
> GNOME_COMPILE_WARNINGS is the place for cosmetic additions. I have been
> subjected to the colourful warnings on Fedora 20 and find them garish at best.
> There is at least one other gnome-common bugs about cosmetic changes, such as
> enabling Automake silent rules in bug 580062, so it might make sense to have a
> macro for applying some nice style options, but I think it would have to be
> opt-in, and distinct from compiler warnings.

I put it in GNOME_COMPILE_WARNINGS because it’s a flag which only affects (the reporting of) compile warnings. I see your point, though.

How about expanding GNOME_COMPILE_WARNINGS to have a --enable-compiler-colors option (or something like that; better name suggestions welcome)? I’m reluctant to add a whole new macro because that greatly increases the barrier for adoption: instead of adding --enable-compiler-colors to autogenargs in their .jhbuildrc, everyone would now also have to add a macro invocation in all their modules, and if, for example, I wanted to use compiler colours on a module I had no commit rights to, I couldn’t (without manually setting -fdiagnostics-color=auto, which sort of goes against the point of this).

In fact, if people are going to have to opt-in to this, why don’t we not put it in gnome-common at all, and just recommend that people opt-in by adding -fdiagnostics-color=auto to their CFLAGS in .jhbuildrc?
Comment 9 Philip Withnall 2013-12-20 22:43:03 UTC
Thoughts?
Comment 10 David King 2013-12-21 21:46:41 UTC
Sorry for the delay. I think this is best off outside of gnome-common. It is quite easy to add this on a per-user basis, as you described, so I do not think that there is much to gain by adding another argument to GNOME_COMPILE_WARNINGS for something that is cosmetic. That seems to be something which the user should set as desired. Adding the flag to the sample jhbuildrc might be a good place.
Comment 11 Philip Withnall 2013-12-22 11:21:04 UTC
(In reply to comment #10)
> Adding the flag to the sample jhbuildrc might be a good
> place.

https://bugzilla.gnome.org/show_bug.cgi?id=720925

Thanks for the feedback. :-)