[FFmpeg-devel] [PATCH] add colours to warnings and errors

[Michael Niedermayer]

On Mon, Apr 26, 2010 at 11:57:34AM +0200, James Darnley wrote:
> On 26 April 2010 11:28, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Mon, Apr 26, 2010 at 12:22:08AM +0200, James Darnley wrote:
> >> On 25 April 2010 23:59, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> > On Sun, Apr 25, 2010 at 11:57:08PM +0200, James Darnley wrote:
> >> >> On 25 April 2010 23:21, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> >> > On Sun, Apr 25, 2010 at 10:40:19PM +0200, James Darnley wrote:
> >> >> >> On 25 April 2010 20:29, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> >> >> >> ?log.c | ? 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
> >> >> >> >> ?1 file changed, 71 insertions(+), 7 deletions(-)
> >> >> >> >> bbd24cb9a7eb75393a8fb35fc3ca620a0edb0af7 ?colours.diff
> >> >> >> >> Index: libavutil/log.c
> >> >> >> >> ===================================================================
> >> >> >> >> --- libavutil/log.c ? (revision 22960)
> >> >> >> >> +++ libavutil/log.c ? (working copy)
> >> >> >> >> @@ -24,6 +24,10 @@
> >> >> >> >> ? * logging functions
> >> >> >> >> ? */
> >> >> >> >>
> >> >> >> >> +#ifdef _WIN32
> >> >> >> >> +#include <windows.h>
> >> >> >> >> +#include <string.h>
> >> >> >> >> +#endif
> >> >> >> >> ?#include <unistd.h>
> >> >> >> >> ?#include <stdlib.h>
> >> >> >> >> ?#include "avutil.h"
> >> >> >> >> @@ -34,24 +38,84 @@
> >> >> >> >> ?#endif
> >> >> >> >> ?int av_log_level = AV_LOG_INFO;
> >> >> >> >>
> >> >> >> >> +/* FIXME: On Windows isatty() returns true when ANSI color codes won't work.
> >> >> >> >> +Some hack to detect output to other terminals would be good, fixing the other
> >> >> >> >> +terminals would be better. One probable exception is when the user has
> >> >> >> >> +ANSI.SYS loaded but the Windows API should then still work. */
> >> >> >> >> +
> >> >> >> >
> >> >> >> >> +#if !HAVE_ISATTY
> >> >> >> >> +#define isatty(2) 0
> >> >> >> >> +#endif
> >> >> >> >
> >> >> >> > does HAVE_ISATTY && isatty(2) work? if so this is prefered over the ifdeffery
> >> >> >> >
> >> >> >>
> >> >> >> I've got no idea because I don't have a system without it. ?But surely
> >> >> >> if you don't have it trying to use isatty() will result in a compile
> >> >> >> error. ?I'm just moving your code about there (and adding a not).
> >> >> >
> >> >> > the compiler should optimize the call away, we use such code elsewhere.
> >> >> > If you want to argue this violates ISO C then you have a point but
> >> >> > its a point against using this anywhere not a point against this specific
> >> >> > case.
> >> >> > Thus as it stands please use it or start a thread asking us to remove it
> >> >> > everywhere. The intermediate of using this trick at some places and not
> >> >> > at others is surely a bad idea.
> >> >>
> >> >> Sorry, I slightly misread what you typed the first time. ?Yes, my test
> >> >> with the following does not result in a compile error only a warning
> >> >> of "implicit declaration of function"
> >> >> #undef HAVE_ISATTY
> >> >> #define HAVE_ISATTY 0
> >> >> #define isatty(x) this_is_not_a_function( x )
> >> >>
> >> >> No, I don't want to argue about correctness. ?I'll leave that to
> >> >> people who actually know what is.
> >> >>
> >> >> Speaking of correctness, can I just get a confirmation if this is an
> >> >> acceptable way to erase the use of a function?
> >> >> #ifndef _WIN32
> >> >> #define SetConsoleTextAttribute(x,y) ;
> >> >> #endif
> >> >
> >> > its not pretty, so if i find an alternative ill likely will ask you to
> >> > change it to that :)
> >> > if i dont (and i dont see one currently) then its fine
> >> >
> >>
> >> Okay. ?I've attached the patch
> >
> 
> >> ? ? ?if(use_ansi_color<0){
> >> -#if HAVE_ISATTY && !defined(_WIN32)
> >> - ? ? ? ?use_ansi_color= getenv("TERM") && !getenv("NO_COLOR") && isatty(2);
> >> -#else
> >> - ? ? ? ?use_ansi_color= 0;
> >> -#endif
> >> + ? ? ? ?use_ansi_color= HAVE_ISATTY && isatty(2) && getenv("TERM") && !getenv("NO_COLOR");
> >
> > as mans pointed out this doesnt work due to configure adding
> > -Werror=missing-prototypes
> > thus we have to use the less readable variant
> >
> 
> Should it be defined as 0 at the top of the file, or should I leave
> your original #if inside the posix case?

i prefer the original

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Incandescent light bulbs waste a lot of energy as heat so the EU forbids them.
Their replacement, compact fluorescent lamps, much more expensive, dont fit in
many old lamps, flicker, contain toxic mercury, produce a fraction of the light
that is claimed and in a unnatural spectrum rendering colors different than
in natural light. Ah and we now need to turn the heaters up more in winter to
compensate the lower wasted heat. Who wins? Not the environment, thats for sure
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100426/361917b1/attachment.pgp>