Skip to content

Commit

Permalink
Print DLite errors in color by default (#748)
Browse files Browse the repository at this point in the history
# Description
By default, print DLite errors in color if the error stream is a
terminal.

The default can be overridden with the ERR_COLOR environment variable.
Like the `--color` option to `ls`, ERR_COLOR can be "always" or "never".
All other values correspond to the default.

## Type of change
- [ ] Bug fix & code cleanup
- [x] New feature
- [ ] Documentation update
- [ ] Test update

## Checklist for the reviewer
This checklist should be used as a help for the reviewer.

- [ ] Is the change limited to one issue?
- [ ] Does this PR close the issue?
- [ ] Is the code easy to read and understand?
- [ ] Do all new feature have an accompanying new test?
- [ ] Has the documentation been updated as necessary?
  • Loading branch information
jesper-friis authored Dec 19, 2023
2 parents 3f260cc + 3dedced commit 811288c
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 6 deletions.
7 changes: 5 additions & 2 deletions doc/user_guide/environment_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,11 @@ Environment variables for controlling error handling
- "4" | "ignore-new" : ignore new error message
- otherwise : append new error message to the old one

- **ERR_COLOR**: If defined, error messages to stdout and stderr will be
written in colour.
- **ERR_COLOR**: Whether to print error messages in colour. May be:
- "0" | "never" : print errors colour-coded
- "1" | "always" : print errors not colour-coded
- otherwise : only print errors colour-coded if the error
stream is a terminal


Path handling when using the pre-packaged wheel (Linux, Windows)
Expand Down
4 changes: 4 additions & 0 deletions src/utils/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ check_include_file(endian.h HAVE_ENDIAN_H)
check_include_file(byteswap.h HAVE_BYTESWAP_H)
check_include_file(locale.h HAVE_LOCALE_H)
check_include_file(time.h HAVE_TIME_H)
check_include_file(unistd.h HAVE_UNISTD_H)
check_include_file(windows.h HAVE_WINDOWS_H)

# -- check for symbols
Expand Down Expand Up @@ -48,6 +49,9 @@ check_symbol_exists(stat sys/stat.h HAVE_STAT)
check_symbol_exists(exec unistd.h HAVE_EXEC)
check_symbol_exists(clock time.h HAVE_CLOCK)
check_symbol_exists(P_tmpdir stdio.h HAVE_P_TMPDIR)
check_symbol_exists(_fileno stdio.h HAVE__FILENO)
check_symbol_exists(isatty unistd.h HAVE_ISATTY)
check_symbol_exists(_isatty io.h HAVE__ISATTY)

if(HAVE_WINDOWS_H)
check_symbol_exists(GetFullPathNameW windows.h HAVE_GetFullPathNameW)
Expand Down
6 changes: 6 additions & 0 deletions src/utils/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#cmakedefine HAVE_INTTYPES_H
#cmakedefine HAVE_LOCALE_H
#cmakedefine HAVE_TIME_H
#cmakedefine HAVE_UNISTD_H
#cmakedefine HAVE_WINDOWS_H

/* Whether symbols exists. If not, they are defined in compat.c or compat/ */
Expand All @@ -55,6 +56,9 @@
#cmakedefine HAVE_EXEC
#cmakedefine HAVE_CLOCK
#cmakedefine HAVE_P_TMPDIR
#cmakedefine HAVE__FILENO
#cmakedefine HAVE_ISATTY
#cmakedefine HAVE__ISATTY

#cmakedefine HAVE_GetFullPathNameW
#cmakedefine HAVE_GetFullPathName
Expand All @@ -66,6 +70,8 @@
#cmakedefine HAVE_CreateProcessA
#cmakedefine HAVE_GetSystemTime
#cmakedefine HAVE_BCryptGenRandom
// #cmakedefine HAVE_GetConsoleMode
// #cmakedefine HAVE_GetStdHandle
#cmakedefine HAVE_MBSTOWCS
#cmakedefine HAVE_WCSTOMBS
#cmakedefine HAVE_MBSTOWCS_S
Expand Down
55 changes: 53 additions & 2 deletions src/utils/err.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
/*
* TODO: Consider to reuse good ideas from https://github.com/rxi/log.c
*/

#include <assert.h>
#include <stdio.h>
#include <stdarg.h>
Expand All @@ -17,6 +16,10 @@
#include "compat.h"
#include "err.h"

#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif

/* Thread local storage
* https://stackoverflow.com/questions/18298280/how-to-declare-a-variable-as-thread-local-portab
*/
Expand Down Expand Up @@ -68,6 +71,9 @@ typedef struct {
* If negative (default), check the environment. */
ErrDebugMode err_debug_mode;

/* Whether to */
ErrColorMode err_color_mode;

/* How to handle overridden errors in ErrTry clauses.
* If negative (default), check the environment. */
ErrOverrideMode err_override;
Expand Down Expand Up @@ -109,6 +115,7 @@ static void reset_tls(void)
_tls.err_abort_mode = -1;
_tls.err_warn_mode = -1;
_tls.err_debug_mode = -1;
_tls.err_color_mode = -1;
_tls.err_override = -1;
_tls.err_record = &_tls.err_root_record;
_tls.globals = &_globals;
Expand All @@ -123,6 +130,7 @@ static ThreadLocals *get_tls(void)
_tls.err_abort_mode = -1;
_tls.err_warn_mode = -1;
_tls.err_debug_mode = -1;
_tls.err_color_mode = -1;
_tls.err_override = -1;
_tls.err_record = &_tls.err_root_record;
_tls.globals = &_globals;
Expand Down Expand Up @@ -619,11 +627,54 @@ ErrOverrideMode err_get_override_mode()
return tls->err_override;
}

ErrColorMode err_set_color_mode(ErrColorMode mode)
{
ThreadLocals *tls = get_tls();
ErrColorMode prev = tls->err_color_mode;
tls->err_color_mode = mode;
return prev;
}

int err_get_color_coded()
{
ThreadLocals *tls = get_tls();
if (tls->err_color_mode < 0) {
char *mode = getenv("ERR_COLOR");
tls->err_color_mode =
(!mode || !*mode) ? errColorAuto :
(strcmp(mode, "never") == 0 || strcmp(mode, "0") == 0) ? errColorNever :
(strcmp(mode, "always") == 0 || strcmp(mode,"1") == 0) ? errColorAlways :
errColorAuto;
}

switch (tls->err_color_mode) {
case errColorAuto:
{
int terminal = 0;
#if defined(HAVE_UNISTD_H) && defined(HAVE_ISATTY)
int fno = (tls->globals->err_stream) ?
fileno(tls->globals->err_stream) : -1;
if (fno >= 0) terminal = (isatty(fno) == 1);
#elif defined(HAVE__FILENO) && defined(HAVE__ISATTY)
int fno = (tls->globals->err_stream) ?
_fileno(tls->globals->err_stream) : -1;
if (fno >= 0) terminal = _isatty(fno);
#endif
return (terminal) ? 1 : 0;
}
case errColorAlways:
return 1;
default:
return 0;
}
}

/* Default error handler. */
void err_default_handler(const ErrRecord *record)
{
FILE *stream = err_get_stream();
if ((stream == stderr || stream == stdout) && getenv("ERR_COLOR")) {
if (err_get_color_coded()) {
//if ((stream == stderr || stream == stdout) && getenv("ERR_COLOR")) {
/* Output the first word of the error message in red and the remaining
of it in magenta. */
int n = strcspn(record->msg, ": ");
Expand Down
27 changes: 25 additions & 2 deletions src/utils/err.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,11 @@
* - "4" | "ignore-new" : ignore new error message
* - otherwise : append new error message to the old one
*
* * `ERR_COLOR`: If defined, error messages to stdout and stderr will be
* written in colour.
* * `ERR_COLOR`: Whether to write error messages colour-coded.
* - unset | "auto" : colour-coded only if connected to a terminal
* - "no" | "never" : not colour-coded
* - "yes" | "always" : colour-coded
*
*/

/** @cond private */
Expand Down Expand Up @@ -131,6 +134,14 @@ typedef enum {
errDebugFull /*!< add also function name to error messages */
} ErrDebugMode;

/** Error color mode */
typedef enum {
errColorEnv=-1, /*!< set from ERR_COLOR environment variable */
errColorAuto=0, /*!< only color-coded if connected to terminal */
errColorAlways, /*!< always color-coded */
errColorNever, /*!< never color-coded */
} ErrColorMode;

/** Error override mode */
typedef enum {
errOverrideEnv=-1, /*!< set from ERR_OVERRIDE environment variable */
Expand Down Expand Up @@ -465,6 +476,18 @@ ErrOverrideMode err_set_override_mode(int mode);
*/
ErrOverrideMode err_get_override_mode(void);


/**
* @brief Sets whether to print error messages color-coded.
*/
ErrColorMode err_set_color_mode(ErrColorMode mode);

/**
* @brief Returns whether error messages are printed color-coded.
*/
int err_get_color_coded();


/* Where in an ErrTry.. ErrEnd clause we are */
typedef enum {
errTryNormal,
Expand Down

0 comments on commit 811288c

Please sign in to comment.