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

[Michael Niedermayer]

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.


> 
> >
> >> +#ifndef _WIN32
> >> +#define SetConsoleTextAttribute(x,y) ;
> >> +#endif
> >> +
> >> ?static int use_ansi_color=-1;
> >> +static int use_win_color=-1;
> >>
> >> ?#undef fprintf
> >> ?static void colored_fputs(int color, const char *str){
> >> +#ifdef _WIN32
> >> + ? ?static int16_t attr, attr_orig;
> >> + ? ?static HANDLE console;
> >> + ? ?static const int16_t win_color_conv[] = {
> >> + ? ? ? ?0,
> >> + ? ? ? ?FOREGROUND_RED,
> >> + ? ? ? ?FOREGROUND_GREEN,
> >> + ? ? ? ?FOREGROUND_RED ?|FOREGROUND_GREEN,
> >> + ? ? ? ?FOREGROUND_BLUE,
> >> + ? ? ? ?FOREGROUND_RED ?|FOREGROUND_BLUE,
> >> + ? ? ? ?FOREGROUND_GREEN|FOREGROUND_BLUE,
> >> + ? ? ? ?FOREGROUND_RED ?|FOREGROUND_GREEN|FOREGROUND_BLUE
> >> + ? ?};
> >> +
> >
> >> + ? ?if (use_ansi_color<0 || use_win_color<0) {
> >
> > these checks are redundant
> >
> 
> What?  It's only what you do in the POSIX case.  Or do you mean that
> it should be an and?

i mean that both will be < 0 or both will be >=0 thus checking for both
is unneeded

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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- 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/20100425/ad2b8b60/attachment.pgp>