From e608bf6f7f95de208ca95f9d2d782203dd78e1ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 26 Apr 2019 08:46:01 +0200 Subject: [PATCH 1/4] run: reword a comment Use unicode m-dash and remove the part about discoverability, since it's just a warning on the terminal. --- src/run/run.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index babd9027e4..f200b28dfe 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -390,11 +390,10 @@ static int parse_argv(int argc, char *argv[]) { r = calendar_spec_next_usec(cs, now(CLOCK_REALTIME), NULL); if (r == -ENOENT) - /* The calendar event is in the past - let's warn about this, but install it - * anyway as it is. The service manager will trigger the service right-away - * then, but everything is discoverable as usual. Moreover, the server side - * might have a different clock or timezone than we do, hence it should - * decide when or whether to run something. */ + /* The calendar event is in the past — let's warn about this, but install it + * anyway as is. The service manager will trigger the service right away. + * Moreover, the server side might have a different clock or timezone than we + * do, hence it should decide when or whether to run something. */ log_warning("Specified calendar expression is in the past, proceeding anyway."); else if (r < 0) return log_error_errno(r, "Failed to calculate next time calendar expression elapses: %m"); From 37b8d2f699ac6c80d653e94bad638122a6a1b3ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 26 Apr 2019 12:27:33 +0200 Subject: [PATCH 2/4] basic/log: use colors to highlight messages like journalctl --- src/basic/log.c | 13 +++++++------ src/basic/terminal-util.c | 30 ++++++++++++++++++++++++++++++ src/basic/terminal-util.h | 3 +++ src/shared/logs-show.c | 17 ++--------------- 4 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/basic/log.c b/src/basic/log.c index ea252c4130..a81c350ab4 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -336,7 +336,7 @@ static int write_to_console( char location[256], prefix[1 + DECIMAL_STR_MAX(int) + 2]; struct iovec iovec[6] = {}; - bool highlight; + const char *on = NULL, *off = NULL; size_t n = 0; if (console_fd < 0) @@ -347,18 +347,19 @@ static int write_to_console( iovec[n++] = IOVEC_MAKE_STRING(prefix); } - highlight = LOG_PRI(level) <= LOG_ERR && show_color; + if (show_color) + get_log_colors(LOG_PRI(level), &on, &off, NULL); if (show_location) { (void) snprintf(location, sizeof location, "(%s:%i) ", file, line); iovec[n++] = IOVEC_MAKE_STRING(location); } - if (highlight) - iovec[n++] = IOVEC_MAKE_STRING(ANSI_HIGHLIGHT_RED); + if (on) + iovec[n++] = IOVEC_MAKE_STRING(on); iovec[n++] = IOVEC_MAKE_STRING(buffer); - if (highlight) - iovec[n++] = IOVEC_MAKE_STRING(ANSI_NORMAL); + if (off) + iovec[n++] = IOVEC_MAKE_STRING(off); iovec[n++] = IOVEC_MAKE_STRING("\n"); if (writev(console_fd, iovec, n) < 0) { diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index b692c52e59..4676ce2a80 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1310,3 +1310,33 @@ int vt_release(int fd, bool restore) { return 0; } + +void get_log_colors(int priority, const char **on, const char **off, const char **highlight) { + /* Note that this will initialize output variables only when there's something to output. + * The caller must pre-initalize to "" or NULL as appropriate. */ + + if (priority <= LOG_ERR) { + if (on) + *on = ANSI_HIGHLIGHT_RED; + if (off) + *off = ANSI_NORMAL; + if (highlight) + *highlight = ANSI_HIGHLIGHT; + + } else if (priority <= LOG_NOTICE) { + if (on) + *on = ANSI_HIGHLIGHT; + if (off) + *off = ANSI_NORMAL; + if (highlight) + *highlight = ANSI_HIGHLIGHT_RED; + + } else if (priority >= LOG_DEBUG) { + if (on) + *on = ANSI_GREY; + if (off) + *off = ANSI_NORMAL; + if (highlight) + *highlight = ANSI_HIGHLIGHT_RED; + } +} diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h index c885e0a2d1..f3e785e7ba 100644 --- a/src/basic/terminal-util.h +++ b/src/basic/terminal-util.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "macro.h" @@ -158,3 +159,5 @@ int vt_default_utf8(void); int vt_reset_keyboard(int fd); int vt_restore(int fd); int vt_release(int fd, bool restore_vt); + +void get_log_colors(int priority, const char **on, const char **off, const char **highlight); diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index 5fb736f633..bdde6c11e3 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -163,21 +163,8 @@ static bool print_multiline( bool ellipsized = false; int line = 0; - if (flags & OUTPUT_COLOR) { - if (priority <= LOG_ERR) { - color_on = ANSI_HIGHLIGHT_RED; - color_off = ANSI_NORMAL; - highlight_on = ANSI_HIGHLIGHT; - } else if (priority <= LOG_NOTICE) { - color_on = ANSI_HIGHLIGHT; - color_off = ANSI_NORMAL; - highlight_on = ANSI_HIGHLIGHT_RED; - } else if (priority >= LOG_DEBUG) { - color_on = ANSI_GREY; - color_off = ANSI_NORMAL; - highlight_on = ANSI_HIGHLIGHT_RED; - } - } + if (flags & OUTPUT_COLOR) + get_log_colors(priority, &color_on, &color_off, &highlight_on); /* A special case: make sure that we print a newline when the message is empty. */ From 1a04395959ca8cdca91f9da86b8580ef27025a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 26 Apr 2019 12:28:25 +0200 Subject: [PATCH 3/4] Enable log colors for most of tools in /usr/bin When emitting the calendarspec warning we want to see some color. Follow-up for 04220fda5c. Exceptions: - systemctl, because it has a lot hand-crafted coloring - tmpfiles, sysusers, stdio-bridge, etc, because they are also used in services and I'm not sure if this wouldn't mess up something. --- src/activate/activate.c | 1 + src/analyze/analyze.c | 1 + src/ask-password/ask-password.c | 1 + src/busctl/busctl.c | 1 + src/cgls/cgls.c | 1 + src/cgtop/cgtop.c | 1 + src/coredump/coredumpctl.c | 1 + src/delta/delta.c | 1 + src/detect-virt/detect-virt.c | 1 + src/escape/escape.c | 1 + src/hostname/hostnamectl.c | 1 + src/id128/id128.c | 1 + src/journal/cat.c | 1 + src/journal/journalctl.c | 1 + src/locale/localectl.c | 1 + src/login/inhibit.c | 1 + src/login/loginctl.c | 1 + src/machine/machinectl.c | 1 + src/mount/mount-tool.c | 1 + src/network/networkctl.c | 1 + src/notify/notify.c | 1 + src/path/path.c | 1 + src/portable/portablectl.c | 1 + src/resolve/resolvectl.c | 1 + src/run/run.c | 1 + src/timedate/timedatectl.c | 1 + 26 files changed, 26 insertions(+) diff --git a/src/activate/activate.c b/src/activate/activate.c index 48aa617e78..8bdf2e50aa 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -474,6 +474,7 @@ int main(int argc, char **argv, char **envp) { int r, n; int epoll_fd = -1; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 97d94516c8..09d26da0db 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -2104,6 +2104,7 @@ static int run(int argc, char *argv[]) { setlocale(LC_ALL, ""); setlocale(LC_NUMERIC, "C"); /* we want to format/parse floats in C style */ + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c index 4637c32819..ca6da40bbc 100644 --- a/src/ask-password/ask-password.c +++ b/src/ask-password/ask-password.c @@ -158,6 +158,7 @@ static int run(int argc, char *argv[]) { char **p; int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 86efc02bd8..e702f9366a 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -2508,6 +2508,7 @@ static int busctl_main(int argc, char *argv[]) { static int run(int argc, char *argv[]) { int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/cgls/cgls.c b/src/cgls/cgls.c index 09ee0e86a2..adc028bc8e 100644 --- a/src/cgls/cgls.c +++ b/src/cgls/cgls.c @@ -165,6 +165,7 @@ static void show_cg_info(const char *controller, const char *path) { static int run(int argc, char *argv[]) { int r, output_flags; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index f27c80120d..de958b57dd 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -909,6 +909,7 @@ static int run(int argc, char *argv[]) { CGroupMask mask; int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index b239d81945..e8aaa50235 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -1069,6 +1069,7 @@ static int run(int argc, char *argv[]) { int r, units_active; setlocale(LC_ALL, ""); + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/delta/delta.c b/src/delta/delta.c index c28a816d67..63baa74794 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -639,6 +639,7 @@ static int parse_argv(int argc, char *argv[]) { static int run(int argc, char *argv[]) { int r, k, n_found = 0; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/detect-virt/detect-virt.c b/src/detect-virt/detect-virt.c index f05f2279b3..7fb80ca138 100644 --- a/src/detect-virt/detect-virt.c +++ b/src/detect-virt/detect-virt.c @@ -128,6 +128,7 @@ static int run(int argc, char *argv[]) { * to detect whether we are being run in a virtualized * environment or not */ + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/escape/escape.c b/src/escape/escape.c index 8b38d9f679..9066c30853 100644 --- a/src/escape/escape.c +++ b/src/escape/escape.c @@ -159,6 +159,7 @@ static int run(int argc, char *argv[]) { char **i; int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index 7f9bb49e0c..22de7350fe 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -432,6 +432,7 @@ static int run(int argc, char *argv[]) { int r; setlocale(LC_ALL, ""); + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/id128/id128.c b/src/id128/id128.c index 60551ec225..4b370d700d 100644 --- a/src/id128/id128.c +++ b/src/id128/id128.c @@ -155,6 +155,7 @@ static int id128_main(int argc, char *argv[]) { static int run(int argc, char *argv[]) { int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/journal/cat.c b/src/journal/cat.c index 9900bd2e86..c28f3b6d80 100644 --- a/src/journal/cat.c +++ b/src/journal/cat.c @@ -127,6 +127,7 @@ static int run(int argc, char *argv[]) { _cleanup_close_ int outfd = -1, errfd = -1, saved_stderr = -1; int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 00489098ee..3479535ba7 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -2100,6 +2100,7 @@ int main(int argc, char *argv[]) { int n_shown = 0, r, poll_fd = -1; setlocale(LC_ALL, ""); + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/locale/localectl.c b/src/locale/localectl.c index c8b195d9a6..9fb5152110 100644 --- a/src/locale/localectl.c +++ b/src/locale/localectl.c @@ -510,6 +510,7 @@ static int run(int argc, char *argv[]) { int r; setlocale(LC_ALL, ""); + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/login/inhibit.c b/src/login/inhibit.c index 47fd9e874d..d96c8195a2 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -270,6 +270,7 @@ static int run(int argc, char *argv[]) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/login/loginctl.c b/src/login/loginctl.c index 342ac56d90..fcd5e169a9 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -1529,6 +1529,7 @@ static int run(int argc, char *argv[]) { int r; setlocale(LC_ALL, ""); + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index d98027a0ca..51bad267d8 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -3100,6 +3100,7 @@ static int run(int argc, char *argv[]) { int r; setlocale(LC_ALL, ""); + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index b290095b0e..fcc4d4da61 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -1525,6 +1525,7 @@ static int run(int argc, char* argv[]) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/network/networkctl.c b/src/network/networkctl.c index 3881aaa42b..4c51ad6a89 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -1128,6 +1128,7 @@ static void warn_networkd_missing(void) { static int run(int argc, char* argv[]) { int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/notify/notify.c b/src/notify/notify.c index fff5233b0c..0b518e1604 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -151,6 +151,7 @@ static int run(int argc, char* argv[]) { unsigned i = 0; int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/path/path.c b/src/path/path.c index 7b630a5714..568b5fb04b 100644 --- a/src/path/path.c +++ b/src/path/path.c @@ -171,6 +171,7 @@ static int parse_argv(int argc, char *argv[]) { static int run(int argc, char* argv[]) { int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index 2c59f04eb5..5041745b3d 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -956,6 +956,7 @@ static int run(int argc, char *argv[]) { int r; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 1d2fc71309..9637e00e60 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -3137,6 +3137,7 @@ static int run(int argc, char **argv) { int r; setlocale(LC_ALL, ""); + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/run/run.c b/src/run/run.c index f200b28dfe..05c1552a8b 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1652,6 +1652,7 @@ static int run(int argc, char* argv[]) { _cleanup_free_ char *description = NULL; int r, retval = EXIT_SUCCESS; + log_show_color(true); log_parse_environment(); log_open(); diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index 406ca1df66..e03f1796e7 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -866,6 +866,7 @@ static int run(int argc, char *argv[]) { int r; setlocale(LC_ALL, ""); + log_show_color(true); log_parse_environment(); log_open(); From 0d0464d39d78fc7db682313f14effb384da2256f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 May 2019 17:42:46 +0200 Subject: [PATCH 4/4] journalctl,shared/log: use yellow for warnings In program output, highlighting warnings with ANSI_HIGHLIGHT is not enough, because it doesn't stand out enough. Yellow is more appropriate. I was worried that yellow wouldn't be visible on white background, but (at least gnome-terminal) uses a fairly dark yellow that is fully legible on white and light-colored backgrounds. We also used yellow in many places, e.g. systemctl, so this should be fine. Note: yellow is unreadable on urxvt with white background (urxvt +rv). But grey, which we already used, is also unreadable, so urxvt users would have to disable colors anyway, so this change does not make the problem intrinsically worse. See https://github.com/systemd/systemd/issues/12482#issuecomment-490374210. --- src/basic/terminal-util.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 4676ce2a80..7d9e5d41ab 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1323,6 +1323,14 @@ void get_log_colors(int priority, const char **on, const char **off, const char if (highlight) *highlight = ANSI_HIGHLIGHT; + } else if (priority <= LOG_WARNING) { + if (on) + *on = ANSI_HIGHLIGHT_YELLOW; + if (off) + *off = ANSI_NORMAL; + if (highlight) + *highlight = ANSI_HIGHLIGHT; + } else if (priority <= LOG_NOTICE) { if (on) *on = ANSI_HIGHLIGHT;