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

[James Darnley]

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).

>
>> +#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?

>
>> + ? ? ? ?CONSOLE_SCREEN_BUFFER_INFO con_info;
>> + ? ? ? ?char *term = getenv("TERM") ? getenv("TERM") : "";
>> +
>> + ? ? ? ?use_ansi_color = isatty(2) && !getenv("NO_COLOR")
>> + ? ? ? ? ? ?&& ( ? !strncmp( term, "msys", ? 4 )
>> + ? ? ? ? ? ? ? ?|| !strncmp( term, "xterm", ?5 )
>> + ? ? ? ? ? ? ? ?|| !strncmp( term, "rxvt", ? 4 )
>> + ? ? ? ? ? ? ? ?//|| !strncmp( getenv("TERM"), "cygwin", 6 )
>> +/* The CYGWIN environment variable makes this hard for native executables.
>> +notty -- programs are directly connected to cmd so the Windows API works
>> +tty -- programs are not directly connected so ANSI color codes work
>> +The default is notty so leaving "cygwin" excluded doesn't cause problems. */
>> + ? ? ? ? ? ?);
>> +
>> + ? ? ? ?console = GetStdHandle( STD_ERROR_HANDLE );
>> + ? ? ? ?if (console != INVALID_HANDLE_VALUE && !use_ansi_color) {
>> + ? ? ? ? ? ?GetConsoleScreenBufferInfo( console, &con_info );
>> + ? ? ? ? ? ?attr_orig = con_info.wAttributes;
>> +
>> + ? ? ? ? ? ?attr = attr_orig&0xF0;
>> + ? ? ? ? ? ?attr |= ( attr_orig&BACKGROUND_INTENSITY )?0:FOREGROUND_INTENSITY;
>> +
>> + ? ? ? ? ? ?use_win_color = 1;
>> + ? ? ? ?} else
>> + ? ? ? ? ? ?use_win_color = 0;
>> + ? ?}
>> +#else
>> ? ? ?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;
>> + ? ? ? ?use_ansi_color = isatty(2) && getenv("TERM") && !getenv("NO_COLOR");
>> + ? ? ? ?use_win_color = 0;
>> + ? ?}
>> ?#endif
>
> this else endif is uneeded
> if(use_ansi_color<0){
> ? ?POSIX case
> #ifdef _WIN32
> ? ?windows case
> #endif
> }
> requires less idefery
>

Can't do that.  I need the HANDLE console variable somewhere I can use
in the rest of the function.  That's only one #else less anyway