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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/60434854/attachment.pgp>