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

[Michael Niedermayer]

On Sun, Apr 25, 2010 at 05:04:33PM +0200, James Darnley wrote:
> On 25 April 2010 00:45, James Darnley <james.darnley at gmail.com> wrote:
> > On 25 April 2010 00:00, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> On Sat, Apr 24, 2010 at 11:35:31PM +0200, James Darnley wrote:
> >>> On 24 April 2010 23:18, Michael Niedermayer <michaelni at gmx.at> wrote:
> >>>
> >>> >
> >>> > ? ?if(use_win_color){
> >>> > ? ?...
> >>> > ? ?}else
> >>> > #else
> >>> > ? ?{
> >>> > ? ? ? ?if(use_ansi_color<0){
> >>> > ? ?#if HAVE_ISATTY && !defined(_WIN32)
> >>> > ? ? ? ? ? ?use_ansi_color= getenv("TERM") && !getenv("NO_COLOR") && isatty(2);
> >>> > @@ -53,6 +121,7 @@
> >>> > ? ? ? ?if(use_ansi_color){
> >>> > ? ? ? ? ? ?fprintf(stderr, "\033[0m");
> >>> > ? ? ? ?}
> >>> > ? ?}
> >>> > #endif
> >>> >
> >>>
> >>> Was this a hint that you want if(use_win_color) before if(use_ansi_color)?
> >>
> >> no, i just meant to reduce the ifdefery and reduce the duplication of the
> >> ansi printing
> >
> > As far as I can see, reducing the duplication would mean one more
> > ifdef for the "colour reset function"
> >
> 
> I've reduced the duplication by adding:
> #ifndef _WIN32
> #define SetConsoleTextAttribute(x,y) ;
> #endif
> so that this function becomes nothing.

>  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


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


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


> +    if (use_ansi_color) {
> +        fprintf(stderr, "\033[%d;3%dm", color>>4, color&15);
> +    } else if (use_win_color) {
> +        SetConsoleTextAttribute( console,
> +            (color&15)==9 ? attr_orig : attr|win_color_conv[color&15]
> +        );
>      }
>  
> -    if(use_ansi_color){

cosmetics


> -        fprintf(stderr, "\033[%d;3%dm", color>>4, color&15);
> -    }

>      fputs(str, stderr);
> +
>      if(use_ansi_color){

cosmetics


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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- 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/ed6bf446/attachment.pgp>