Merge branch 'js/colored-push-errors'
authorJunio C Hamano <gitster@pobox.com>
Tue, 8 May 2018 06:59:34 +0000 (15:59 +0900)
committerJunio C Hamano <gitster@pobox.com>
Tue, 8 May 2018 06:59:34 +0000 (15:59 +0900)
Error messages from "git push" can be painted for more visibility.

* js/colored-push-errors:
config: document the settings to colorize push errors/hints
push: test to verify that push errors are colored
push: colorize errors
color: introduce support for colorizing stderr

Documentation/config.txt
advice.c
builtin/push.c
color.c
color.h
config.c
t/t5541-http-push-smart.sh
transport.c
index 8213c1236b19b30efa960d561dcb9ae52254d114..a05a88fec10eb3c4c36b94889ac6d58b8f441cf7 100644 (file)
@@ -1098,6 +1098,16 @@ clean.requireForce::
        A boolean to make git-clean do nothing unless given -f,
        -i or -n.   Defaults to true.
 
+color.advice::
+       A boolean to enable/disable color in hints (e.g. when a push
+       failed, see `advice.*` for a list).  May be set to `always`,
+       `false` (or `never`) or `auto` (or `true`), in which case colors
+       are used only when the error output goes to a terminal. If
+       unset, then the value of `color.ui` is used (`auto` by default).
+
+color.advice.hint::
+       Use customized color for hints.
+
 color.branch::
        A boolean to enable/disable color in the output of
        linkgit:git-branch[1]. May be set to `always`,
@@ -1200,6 +1210,15 @@ color.pager::
        A boolean to enable/disable colored output when the pager is in
        use (default is true).
 
+color.push::
+       A boolean to enable/disable color in push errors. May be set to
+       `always`, `false` (or `never`) or `auto` (or `true`), in which
+       case colors are used only when the error output goes to a terminal.
+       If unset, then the value of `color.ui` is used (`auto` by default).
+
+color.push.error::
+       Use customized color for push errors.
+
 color.showBranch::
        A boolean to enable/disable color in the output of
        linkgit:git-show-branch[1]. May be set to `always`,
@@ -1228,6 +1247,15 @@ color.status.<slot>::
        status short-format), or
        `unmerged` (files which have unmerged changes).
 
+color.transport::
+       A boolean to enable/disable color when pushes are rejected. May be
+       set to `always`, `false` (or `never`) or `auto` (or `true`), in which
+       case colors are used only when the error output goes to a terminal.
+       If unset, then the value of `color.ui` is used (`auto` by default).
+
+color.transport.rejected::
+       Use customized color when a push was rejected.
+
 color.ui::
        This variable determines the default value for variables such
        as `color.diff` and `color.grep` that control the use of color
index 406efc183ba272b94a39d4cc7e97d7483a40a9d4..89fda1de55bfc80ba884ab45dd7a31087dea66df 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "config.h"
+#include "color.h"
 
 int advice_push_update_rejected = 1;
 int advice_push_non_ff_current = 1;
@@ -20,6 +21,33 @@ int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
 
+static int advice_use_color = -1;
+static char advice_colors[][COLOR_MAXLEN] = {
+       GIT_COLOR_RESET,
+       GIT_COLOR_YELLOW,       /* HINT */
+};
+
+enum color_advice {
+       ADVICE_COLOR_RESET = 0,
+       ADVICE_COLOR_HINT = 1,
+};
+
+static int parse_advise_color_slot(const char *slot)
+{
+       if (!strcasecmp(slot, "reset"))
+               return ADVICE_COLOR_RESET;
+       if (!strcasecmp(slot, "hint"))
+               return ADVICE_COLOR_HINT;
+       return -1;
+}
+
+static const char *advise_get_color(enum color_advice ix)
+{
+       if (want_color_stderr(advice_use_color))
+               return advice_colors[ix];
+       return "";
+}
+
 static struct {
        const char *name;
        int *preference;
@@ -59,7 +87,10 @@ void advise(const char *advice, ...)
 
        for (cp = buf.buf; *cp; cp = np) {
                np = strchrnul(cp, '\n');
-               fprintf(stderr, _("hint: %.*s\n"), (int)(np - cp), cp);
+               fprintf(stderr, _("%shint: %.*s%s\n"),
+                       advise_get_color(ADVICE_COLOR_HINT),
+                       (int)(np - cp), cp,
+                       advise_get_color(ADVICE_COLOR_RESET));
                if (*np)
                        np++;
        }
@@ -68,9 +99,23 @@ void advise(const char *advice, ...)
 
 int git_default_advice_config(const char *var, const char *value)
 {
-       const char *k;
+       const char *k, *slot_name;
        int i;
 
+       if (!strcmp(var, "color.advice")) {
+               advice_use_color = git_config_colorbool(var, value);
+               return 0;
+       }
+
+       if (skip_prefix(var, "color.advice.", &slot_name)) {
+               int slot = parse_advise_color_slot(slot_name);
+               if (slot < 0)
+                       return 0;
+               if (!value)
+                       return config_error_nonbool(var);
+               return color_parse(value, advice_colors[slot]);
+       }
+
        if (!skip_prefix(var, "advice.", &k))
                return 0;
 
index 013c20d6164f61dc404b89271c0281d28b5069a7..ac3705370e12fda53da086f33c6b47925a48873c 100644 (file)
 #include "submodule.h"
 #include "submodule-config.h"
 #include "send-pack.h"
+#include "color.h"
 
 static const char * const push_usage[] = {
        N_("git push [<options>] [<repository> [<refspec>...]]"),
        NULL,
 };
 
+static int push_use_color = -1;
+static char push_colors[][COLOR_MAXLEN] = {
+       GIT_COLOR_RESET,
+       GIT_COLOR_RED,  /* ERROR */
+};
+
+enum color_push {
+       PUSH_COLOR_RESET = 0,
+       PUSH_COLOR_ERROR = 1
+};
+
+static int parse_push_color_slot(const char *slot)
+{
+       if (!strcasecmp(slot, "reset"))
+               return PUSH_COLOR_RESET;
+       if (!strcasecmp(slot, "error"))
+               return PUSH_COLOR_ERROR;
+       return -1;
+}
+
+static const char *push_get_color(enum color_push ix)
+{
+       if (want_color_stderr(push_use_color))
+               return push_colors[ix];
+       return "";
+}
+
 static int thin = 1;
 static int deleterefs;
 static const char *receivepack;
@@ -337,8 +365,11 @@ static int push_with_options(struct transport *transport, int flags)
                fprintf(stderr, _("Pushing to %s\n"), transport->url);
        err = transport_push(transport, refspec_nr, refspec, flags,
                             &reject_reasons);
-       if (err != 0)
+       if (err != 0) {
+               fprintf(stderr, "%s", push_get_color(PUSH_COLOR_ERROR));
                error(_("failed to push some refs to '%s'"), transport->url);
+               fprintf(stderr, "%s", push_get_color(PUSH_COLOR_RESET));
+       }
 
        err |= transport_disconnect(transport);
        if (!err)
@@ -467,6 +498,7 @@ static void set_push_cert_flags(int *flags, int v)
 
 static int git_push_config(const char *k, const char *v, void *cb)
 {
+       const char *slot_name;
        int *flags = cb;
        int status;
 
@@ -514,6 +546,16 @@ static int git_push_config(const char *k, const char *v, void *cb)
                        else
                                string_list_append(&push_options_config, v);
                return 0;
+       } else if (!strcmp(k, "color.push")) {
+               push_use_color = git_config_colorbool(k, v);
+               return 0;
+       } else if (skip_prefix(k, "color.push.", &slot_name)) {
+               int slot = parse_push_color_slot(slot_name);
+               if (slot < 0)
+                       return 0;
+               if (!v)
+                       return config_error_nonbool(k);
+               return color_parse(v, push_colors[slot]);
        }
 
        return git_default_config(k, v, NULL);
diff --git a/color.c b/color.c
index f277e72e4ce04815f71c949dfdf7c89c9462c5b7..c6c6c4f580fe9bde55bd5f433b1bffd3932053f8 100644 (file)
--- a/color.c
+++ b/color.c
@@ -319,18 +319,20 @@ int git_config_colorbool(const char *var, const char *value)
        return GIT_COLOR_AUTO;
 }
 
-static int check_auto_color(void)
+static int check_auto_color(int fd)
 {
-       if (color_stdout_is_tty < 0)
-               color_stdout_is_tty = isatty(1);
-       if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
+       static int color_stderr_is_tty = -1;
+       int *is_tty_p = fd == 1 ? &color_stdout_is_tty : &color_stderr_is_tty;
+       if (*is_tty_p < 0)
+               *is_tty_p = isatty(fd);
+       if (*is_tty_p || (fd == 1 && pager_in_use() && pager_use_color)) {
                if (!is_terminal_dumb())
                        return 1;
        }
        return 0;
 }
 
-int want_color(int var)
+int want_color_fd(int fd, int var)
 {
        /*
         * NEEDSWORK: This function is sometimes used from multiple threads, and
@@ -339,15 +341,15 @@ int want_color(int var)
         * is listed in .tsan-suppressions for the time being.
         */
 
-       static int want_auto = -1;
+       static int want_auto[3] = { -1, -1, -1 };
 
        if (var < 0)
                var = git_use_color_default;
 
        if (var == GIT_COLOR_AUTO) {
-               if (want_auto < 0)
-                       want_auto = check_auto_color();
-               return want_auto;
+               if (want_auto[fd] < 0)
+                       want_auto[fd] = check_auto_color(fd);
+               return want_auto[fd];
        }
        return var;
 }
diff --git a/color.h b/color.h
index cd0bcedd084f3741fad55569b18ec15e12d75cf8..5b744e1bc68617d196bdd864042e738cb4d75ebe 100644 (file)
--- a/color.h
+++ b/color.h
@@ -88,7 +88,9 @@ int git_config_colorbool(const char *var, const char *value);
  * Return a boolean whether to use color, where the argument 'var' is
  * one of GIT_COLOR_UNKNOWN, GIT_COLOR_NEVER, GIT_COLOR_ALWAYS, GIT_COLOR_AUTO.
  */
-int want_color(int var);
+int want_color_fd(int fd, int var);
+#define want_color(colorbool) want_color_fd(1, (colorbool))
+#define want_color_stderr(colorbool) want_color_fd(2, (colorbool))
 
 /*
  * Translate a Git color from 'value' into a string that the terminal can
index ec9661413311dfea64637bed021e0ef8ab72b6ec..6f8f1d8c1130f89ccf913cc7f7cafac3dd39e123 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1452,7 +1452,7 @@ int git_default_config(const char *var, const char *value, void *dummy)
        if (starts_with(var, "mailmap."))
                return git_default_mailmap_config(var, value);
 
-       if (starts_with(var, "advice."))
+       if (starts_with(var, "advice.") || starts_with(var, "color.advice"))
                return git_default_advice_config(var, value);
 
        if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) {
index 21340e89c9650e43fda9a6176c6fe814360edb8b..a2af693068fa455838c97df1fefe38bd630ceb2e 100755 (executable)
@@ -377,5 +377,17 @@ test_expect_success 'push status output scrubs password' '
        grep "^To $HTTPD_URL/smart/test_repo.git" status
 '
 
+test_expect_success 'colorize errors/hints' '
+       cd "$ROOT_PATH"/test_repo_clone &&
+       test_must_fail git -c color.transport=always -c color.advice=always \
+               -c color.push=always \
+               push origin origin/master^:master 2>act &&
+       test_decode_color <act >decoded &&
+       test_i18ngrep "<RED>.*rejected.*<RESET>" decoded &&
+       test_i18ngrep "<RED>error: failed to push some refs" decoded &&
+       test_i18ngrep "<YELLOW>hint: " decoded &&
+       test_i18ngrep ! "^hint: " decoded
+'
+
 stop_httpd
 test_done
index 16b2f54f22e910c27ea10c32dfbc398925cc85c1..37410d8aad9dd70df282c133ea9939bba5589efd 100644 (file)
 #include "transport-internal.h"
 #include "protocol.h"
 #include "object-store.h"
+#include "color.h"
+
+static int transport_use_color = -1;
+static char transport_colors[][COLOR_MAXLEN] = {
+       GIT_COLOR_RESET,
+       GIT_COLOR_RED           /* REJECTED */
+};
+
+enum color_transport {
+       TRANSPORT_COLOR_RESET = 0,
+       TRANSPORT_COLOR_REJECTED = 1
+};
+
+static int transport_color_config(void)
+{
+       const char *keys[] = {
+               "color.transport.reset",
+               "color.transport.rejected"
+       }, *key = "color.transport";
+       char *value;
+       int i;
+       static int initialized;
+
+       if (initialized)
+               return 0;
+       initialized = 1;
+
+       if (!git_config_get_string(key, &value))
+               transport_use_color = git_config_colorbool(key, value);
+
+       if (!want_color_stderr(transport_use_color))
+               return 0;
+
+       for (i = 0; i < ARRAY_SIZE(keys); i++)
+               if (!git_config_get_string(keys[i], &value)) {
+                       if (!value)
+                               return config_error_nonbool(keys[i]);
+                       if (color_parse(value, transport_colors[i]) < 0)
+                               return -1;
+               }
+
+       return 0;
+}
+
+static const char *transport_get_color(enum color_transport ix)
+{
+       if (want_color_stderr(transport_use_color))
+               return transport_colors[ix];
+       return "";
+}
 
 static void set_upstreams(struct transport *transport, struct ref *refs,
        int pretend)
@@ -373,7 +423,13 @@ static void print_ref_status(char flag, const char *summary,
                else
                        fprintf(stdout, "%s\n", summary);
        } else {
-               fprintf(stderr, " %c %-*s ", flag, summary_width, summary);
+               const char *red = "", *reset = "";
+               if (push_had_errors(to)) {
+                       red = transport_get_color(TRANSPORT_COLOR_REJECTED);
+                       reset = transport_get_color(TRANSPORT_COLOR_RESET);
+               }
+               fprintf(stderr, " %s%c %-*s%s ", red, flag, summary_width,
+                       summary, reset);
                if (from)
                        fprintf(stderr, "%s -> %s", prettify_refname(from->name), prettify_refname(to->name));
                else
@@ -522,6 +578,9 @@ void transport_print_push_status(const char *dest, struct ref *refs,
        char *head;
        int summary_width = transport_summary_width(refs);
 
+       if (transport_color_config() < 0)
+               warning(_("could not parse transport.color.* config"));
+
        head = resolve_refdup("HEAD", RESOLVE_REF_READING, NULL, NULL);
 
        if (verbose) {
@@ -588,6 +647,9 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
        struct send_pack_args args;
        int ret = 0;
 
+       if (transport_color_config() < 0)
+               return -1;
+
        if (!data->got_remote_heads)
                get_refs_via_connect(transport, 1, NULL);
 
@@ -1036,6 +1098,9 @@ int transport_push(struct transport *transport,
        *reject_reasons = 0;
        transport_verify_remote_names(refspec_nr, refspec);
 
+       if (transport_color_config() < 0)
+               return -1;
+
        if (transport->vtable->push_refs) {
                struct ref *remote_refs;
                struct ref *local_refs = get_local_heads();