From 0d6c12cbc8370dafcfcb2c1882303ab2610175c6 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Sun, 14 Feb 2016 14:42:54 -0500 Subject: Refactor color handling. The main benefit is colored warnings/errors during parsing. --- bfs.h | 6 ++-- color.c | 34 +++++++++++++++----- color.h | 30 ++++++++++++++---- eval.c | 8 +++-- parse.c | 109 ++++++++++++++++++++++++++++++++++++---------------------------- 5 files changed, 122 insertions(+), 65 deletions(-) diff --git a/bfs.h b/bfs.h index 8e879f5..4b43ea5 100644 --- a/bfs.h +++ b/bfs.h @@ -54,8 +54,10 @@ struct cmdline { /** Color data. */ struct color_table *colors; - /** -color option. */ - bool color; + /** Colors to use for stdout. */ + const struct color_table *stdout_colors; + /** Colors to use for stderr. */ + const struct color_table *stderr_colors; /** -mindepth option. */ int mindepth; diff --git a/color.c b/color.c index f32d5a3..aea2a54 100644 --- a/color.c +++ b/color.c @@ -48,6 +48,9 @@ struct color_table { const char *sticky; const char *exec; + const char *warning; + const char *error; + struct ext_color *ext_list; char *data; @@ -79,6 +82,8 @@ struct color_table *parse_colors(const char *ls_colors) { colors->ow = "34;42"; colors->sticky = "37;44"; colors->exec = "01;32"; + colors->warning = "40;33;01"; + colors->error = "40;31;01"; colors->ext_list = NULL; if (ls_colors) { @@ -315,21 +320,36 @@ void pretty_print(const struct color_table *colors, const struct BFTW *ftwbuf) { fputs("\n", stdout); } -void print_error(const struct color_table *colors, const char *path, int error) { - const char *color = NULL; - if (colors) { - color = colors->orphan; - } - +static void pretty_format(const struct color_table *colors, const char *color, const char *format, va_list args) { if (color) { print_esc(color, stderr); } - fprintf(stderr, "'%s': %s\n", path, strerror(error)); + + vfprintf(stderr, format, args); + if (color) { print_esc(colors->reset, stderr); } } +void pretty_warning(const struct color_table *colors, const char *format, ...) { + va_list args; + va_start(args, format); + + pretty_format(colors, colors ? colors->warning : NULL, format, args); + + va_end(args); +} + +void pretty_error(const struct color_table *colors, const char *format, ...) { + va_list args; + va_start(args, format); + + pretty_format(colors, colors ? colors->error : NULL, format, args); + + va_end(args); +} + void free_colors(struct color_table *colors) { if (colors) { struct ext_color *ext = colors->ext_list; diff --git a/color.h b/color.h index 61b71c0..5f66b43 100644 --- a/color.h +++ b/color.h @@ -38,17 +38,35 @@ struct color_table *parse_colors(const char *ls_colors); */ void pretty_print(const struct color_table *colors, const struct BFTW *ftwbuf); +#if __GNUC__ +# define BFS_PRINTF_ATTRIBUTE(f, v) __attribute__((format(printf, f, v))) +#else +# define BFS_PRINTF_ATTRIBUTE(f, v) +#endif + +/** + * Pretty-print a warning message. + * + * @param colors + * The color table to use. + * @param format + * The format string. + * @param ... + * The format string's arguments. + */ +void pretty_warning(const struct color_table *colors, const char *format, ...) BFS_PRINTF_ATTRIBUTE(2, 3); + /** - * Pretty-print an error. + * Pretty-print an error message. * * @param colors * The color table to use. - * @param path - * The file path in error. - * @param error - * The error code that occurred. + * @param format + * The format string. + * @param ... + * The format string's arguments. */ -void print_error(const struct color_table *colors, const char *path, int error); +void pretty_error(const struct color_table *colors, const char *format, ...) BFS_PRINTF_ATTRIBUTE(2, 3); /** * Free a color table. diff --git a/eval.c b/eval.c index 26596f0..125342a 100644 --- a/eval.c +++ b/eval.c @@ -40,7 +40,8 @@ struct eval_state { * Report an error that occurs during evaluation. */ static void eval_error(struct eval_state *state) { - print_error(state->cmdline->colors, state->ftwbuf->path, errno); + pretty_error(state->cmdline->stderr_colors, + "'%s': %s\n", state->ftwbuf->path, strerror(errno)); state->ret = -1; } @@ -386,10 +387,11 @@ bool eval_path(const struct expr *expr, struct eval_state *state) { * -print action. */ bool eval_print(const struct expr *expr, struct eval_state *state) { - struct color_table *colors = state->cmdline->colors; + const struct color_table *colors = state->cmdline->stdout_colors; if (colors) { fill_statbuf(state); } + pretty_print(colors, state->ftwbuf); return true; } @@ -526,7 +528,7 @@ static enum bftw_action cmdline_callback(struct BFTW *ftwbuf, void *ptr) { const struct cmdline *cmdline = args->cmdline; if (ftwbuf->typeflag == BFTW_ERROR) { - print_error(cmdline->colors, ftwbuf->path, ftwbuf->error); + pretty_error(cmdline->stderr_colors, "'%s': %s\n", ftwbuf->path, strerror(ftwbuf->error)); return BFTW_SKIP_SUBTREE; } diff --git a/parse.c b/parse.c index 7951589..e1e3c62 100644 --- a/parse.c +++ b/parse.c @@ -155,12 +155,15 @@ struct parser_state { * Invoke stat() on an argument. */ static int stat_arg(const struct parser_state *state, struct expr *expr, struct stat *sb) { - bool follow = state->cmdline->flags & BFTW_FOLLOW; + const struct cmdline *cmdline = state->cmdline; + + bool follow = cmdline->flags & BFTW_FOLLOW; int flags = follow ? 0 : AT_SYMLINK_NOFOLLOW; int ret = fstatat(AT_FDCWD, expr->sdata, sb, flags); if (ret != 0) { - print_error(NULL, expr->sdata, errno); + pretty_error(cmdline->stderr_colors, + "'%s': %s\n", expr->sdata, strerror(errno)); free_expr(expr); } return ret; @@ -197,7 +200,7 @@ static const char *skip_paths(struct parser_state *state) { /** * Parse an integer. */ -static bool parse_int(const char *str, int *value) { +static bool parse_int(const struct parser_state *state, const char *str, int *value) { char *endptr; long result = strtol(str, &endptr, 10); @@ -213,14 +216,15 @@ static bool parse_int(const char *str, int *value) { return true; bad: - fprintf(stderr, "'%s' is not a valid integer.\n", str); + pretty_error(state->cmdline->stderr_colors, + "error: '%s' is not a valid integer.\n", str); return false; } /** * Parse an integer and a comparison flag. */ -static bool parse_icmp(const char *str, struct expr *expr) { +static bool parse_icmp(const struct parser_state *state, const char *str, struct expr *expr) { switch (str[0]) { case '-': expr->cmp = CMP_LESS; @@ -235,7 +239,7 @@ static bool parse_icmp(const char *str, struct expr *expr) { break; } - return parse_int(str, &expr->idata); + return parse_int(state, str, &expr->idata); } /** @@ -243,10 +247,10 @@ static bool parse_icmp(const char *str, struct expr *expr) { */ static struct expr *new_option(struct parser_state *state, const char *option) { if (state->warn && state->non_option_seen) { - fprintf(stderr, - "The '%s' option applies to the entire command line.\n" - "For clarity, place it before any non-option arguments.\n\n", - option); + pretty_warning(state->cmdline->stderr_colors, + "warning: The '%s' option applies to the entire command line. For clarity, place\n" + "it before any non-option arguments.\n\n", + option); } return &expr_true; @@ -308,7 +312,8 @@ static struct expr *new_action(struct parser_state *state, eval_fn *eval) { static struct expr *parse_test_icmp(struct parser_state *state, const char *test, eval_fn *eval) { const char *arg = state->argv[state->i]; if (!arg) { - fprintf(stderr, "%s needs a value.\n", test); + pretty_error(state->cmdline->stderr_colors, + "error: %s needs a value.\n", test); return NULL; } @@ -316,7 +321,7 @@ static struct expr *parse_test_icmp(struct parser_state *state, const char *test struct expr *expr = new_test(state, eval); if (expr) { - if (!parse_icmp(arg, expr)) { + if (!parse_icmp(state, arg, expr)) { free_expr(expr); expr = NULL; } @@ -330,7 +335,8 @@ static struct expr *parse_test_icmp(struct parser_state *state, const char *test static struct expr *parse_test_sdata(struct parser_state *state, const char *test, eval_fn *eval) { const char *arg = state->argv[state->i]; if (!arg) { - fprintf(stderr, "%s needs a value.\n", test); + pretty_error(state->cmdline->stderr_colors, + "error: %s needs a value.\n", test); return NULL; } @@ -407,13 +413,14 @@ static struct expr *parse_daystart(struct parser_state *state) { static struct expr *parse_depth(struct parser_state *state, const char *option, int *depth) { const char *arg = state->argv[state->i]; if (!arg) { - fprintf(stderr, "%s needs a value.\n", option); + pretty_error(state->cmdline->stderr_colors, + "error: %s needs a value.\n", option); return NULL; } ++state->i; - if (!parse_int(arg, depth)) { + if (!parse_int(state, arg, depth)) { return NULL; } @@ -423,13 +430,14 @@ static struct expr *parse_depth(struct parser_state *state, const char *option, /** * Set the FNM_CASEFOLD flag, if supported. */ -static struct expr *set_fnm_casefold(struct expr *expr, bool casefold) { +static struct expr *set_fnm_casefold(const struct parser_state *state, const char *option, struct expr *expr, bool casefold) { if (expr) { if (casefold) { #ifdef FNM_CASEFOLD expr->idata = FNM_CASEFOLD; #else - fprintf(stderr, "%s is missing platform support.\n", option); + pretty_error(state->cmdline->stderr_colors, + "error: %s is missing platform support.\n", option); free(expr); expr = NULL; #endif @@ -445,7 +453,7 @@ static struct expr *set_fnm_casefold(struct expr *expr, bool casefold) { */ static struct expr *parse_name(struct parser_state *state, const char *option, bool casefold) { struct expr *expr = parse_test_sdata(state, option, eval_name); - return set_fnm_casefold(expr, casefold); + return set_fnm_casefold(state, option, expr, casefold); } /** @@ -453,7 +461,7 @@ static struct expr *parse_name(struct parser_state *state, const char *option, b */ static struct expr *parse_path(struct parser_state *state, const char *option, bool casefold) { struct expr *expr = parse_test_sdata(state, option, eval_path); - return set_fnm_casefold(expr, casefold); + return set_fnm_casefold(state, option, expr, casefold); } /** @@ -461,7 +469,7 @@ static struct expr *parse_path(struct parser_state *state, const char *option, b */ static struct expr *parse_lname(struct parser_state *state, const char *option, bool casefold) { struct expr *expr = parse_test_sdata(state, option, eval_lname); - return set_fnm_casefold(expr, casefold); + return set_fnm_casefold(state, option, expr, casefold); } /** @@ -469,9 +477,9 @@ static struct expr *parse_lname(struct parser_state *state, const char *option, */ static struct expr *parse_noleaf(struct parser_state *state, const char *option) { if (state->warn) { - fprintf(stderr, - "bfs does not apply the optimization that %s inhibits.\n\n", - option); + pretty_warning(state->cmdline->stderr_colors, + "warning: bfs does not apply the optimization that %s inhibits.\n\n", + option); } return new_option(state, option); @@ -503,7 +511,8 @@ static struct expr *parse_samefile(struct parser_state *state, const char *optio static struct expr *parse_type(struct parser_state *state, const char *option, eval_fn *eval) { const char *arg = state->argv[state->i]; if (!arg) { - fprintf(stderr, "%s needs a value.\n", option); + pretty_error(state->cmdline->stderr_colors, + "error: %s needs a value.\n", option); return NULL; } @@ -534,7 +543,8 @@ static struct expr *parse_type(struct parser_state *state, const char *option, e } if (typeflag == BFTW_UNKNOWN || arg[1] != '\0') { - fprintf(stderr, "Unknown type flag '%s'.\n", arg); + pretty_error(state->cmdline->stderr_colors, + "error: Unknown type flag '%s'.\n", arg); return NULL; } @@ -580,33 +590,36 @@ static struct expr *parse_version(struct parser_state *state) { * | ACTION */ static struct expr *parse_literal(struct parser_state *state) { + struct cmdline *cmdline = state->cmdline; + // Paths are already skipped at this point const char *arg = state->argv[state->i++]; if (arg[0] != '-') { - fprintf(stderr, "Expected a predicate; found '%s'.\n", arg); + pretty_error(cmdline->stderr_colors, + "error: Expected a predicate; found '%s'.\n", arg); return NULL; } switch (arg[1]) { case 'P': if (strcmp(arg, "-P") == 0) { - state->cmdline->flags &= ~(BFTW_FOLLOW | BFTW_DETECT_CYCLES); + cmdline->flags &= ~(BFTW_FOLLOW | BFTW_DETECT_CYCLES); return new_option(state, arg); } break; case 'H': if (strcmp(arg, "-H") == 0) { - state->cmdline->flags &= ~(BFTW_FOLLOW_NONROOT | BFTW_DETECT_CYCLES); - state->cmdline->flags |= BFTW_FOLLOW_ROOT; + cmdline->flags &= ~(BFTW_FOLLOW_NONROOT | BFTW_DETECT_CYCLES); + cmdline->flags |= BFTW_FOLLOW_ROOT; return new_option(state, arg); } break; case 'L': if (strcmp(arg, "-L") == 0) { - state->cmdline->flags |= BFTW_FOLLOW | BFTW_DETECT_CYCLES; + cmdline->flags |= BFTW_FOLLOW | BFTW_DETECT_CYCLES; return new_option(state, arg); } break; @@ -629,7 +642,8 @@ static struct expr *parse_literal(struct parser_state *state) { } else if (strcmp(arg, "-cnewer") == 0) { return parse_acnewer(state, arg, CTIME); } else if (strcmp(arg, "-color") == 0) { - state->cmdline->color = true; + cmdline->stdout_colors = cmdline->colors; + cmdline->stderr_colors = cmdline->colors; return new_option(state, arg); } break; @@ -638,10 +652,10 @@ static struct expr *parse_literal(struct parser_state *state) { if (strcmp(arg, "-daystart") == 0) { return parse_daystart(state); } else if (strcmp(arg, "-delete") == 0) { - state->cmdline->flags |= BFTW_DEPTH; + cmdline->flags |= BFTW_DEPTH; return new_action(state, eval_delete); } else if (strcmp(arg, "-d") == 0 || strcmp(arg, "-depth") == 0) { - state->cmdline->flags |= BFTW_DEPTH; + cmdline->flags |= BFTW_DEPTH; return new_option(state, arg); } break; @@ -658,7 +672,7 @@ static struct expr *parse_literal(struct parser_state *state) { if (strcmp(arg, "-false") == 0) { return &expr_false; } else if (strcmp(arg, "-follow") == 0) { - state->cmdline->flags |= BFTW_FOLLOW | BFTW_DETECT_CYCLES; + cmdline->flags |= BFTW_FOLLOW | BFTW_DETECT_CYCLES; return new_option(state, arg); } break; @@ -699,13 +713,13 @@ static struct expr *parse_literal(struct parser_state *state) { case 'm': if (strcmp(arg, "-mindepth") == 0) { - return parse_depth(state, arg, &state->cmdline->mindepth); + return parse_depth(state, arg, &cmdline->mindepth); } else if (strcmp(arg, "-maxdepth") == 0) { - return parse_depth(state, arg, &state->cmdline->maxdepth); + return parse_depth(state, arg, &cmdline->maxdepth); } else if (strcmp(arg, "-mmin") == 0) { return parse_acmtime(state, arg, MTIME, MINUTES); } else if (strcmp(arg, "-mount") == 0) { - state->cmdline->flags |= BFTW_MOUNT; + cmdline->flags |= BFTW_MOUNT; return new_option(state, arg); } else if (strcmp(arg, "-mtime") == 0) { return parse_acmtime(state, arg, MTIME, DAYS); @@ -718,7 +732,8 @@ static struct expr *parse_literal(struct parser_state *state) { } else if (strcmp(arg, "-newer") == 0) { return parse_acnewer(state, arg, MTIME); } else if (strcmp(arg, "-nocolor") == 0) { - state->cmdline->color = false; + cmdline->stdout_colors = NULL; + cmdline->stderr_colors = NULL; return new_option(state, arg); } else if (strcmp(arg, "-nohidden") == 0) { return new_action(state, eval_nohidden); @@ -793,7 +808,7 @@ static struct expr *parse_literal(struct parser_state *state) { case 'x': if (strcmp(arg, "-xdev") == 0) { - state->cmdline->flags |= BFTW_MOUNT; + cmdline->flags |= BFTW_MOUNT; return new_option(state, arg); } else if (strcmp(arg, "-xtype") == 0) { return parse_type(state, arg, eval_xtype); @@ -809,7 +824,8 @@ static struct expr *parse_literal(struct parser_state *state) { break; } - fprintf(stderr, "Unknown argument '%s'.\n", arg); + pretty_error(cmdline->stderr_colors, + "error: Unknown argument '%s'.\n", arg); return NULL; } @@ -1025,13 +1041,15 @@ struct cmdline *parse_cmdline(int argc, char *argv[]) { cmdline->roots = NULL; cmdline->nroots = 0; - cmdline->colors = NULL; - cmdline->color = isatty(STDOUT_FILENO); cmdline->mindepth = 0; cmdline->maxdepth = INT_MAX; cmdline->flags = BFTW_RECOVER; cmdline->expr = &expr_true; + cmdline->colors = parse_colors(getenv("LS_COLORS")); + cmdline->stdout_colors = isatty(STDOUT_FILENO) ? cmdline->colors : NULL; + cmdline->stderr_colors = isatty(STDERR_FILENO) ? cmdline->colors : NULL; + struct parser_state state = { .cmdline = cmdline, .argv = argv, @@ -1059,7 +1077,8 @@ struct cmdline *parse_cmdline(int argc, char *argv[]) { } if (state.i < argc) { - fprintf(stderr, "Unexpected argument '%s'.\n", argv[state.i]); + pretty_error(cmdline->stderr_colors, + "error: Unexpected argument '%s'.\n", argv[state.i]); goto fail; } @@ -1081,10 +1100,6 @@ struct cmdline *parse_cmdline(int argc, char *argv[]) { } } - if (cmdline->color) { - cmdline->colors = parse_colors(getenv("LS_COLORS")); - } - done: return cmdline; -- cgit v1.2.3