On Fri, Jun 13, 2014 at 04:41:05PM +0200, Werner Fink wrote: > Therefore strip off the ANSI escape sequences for 3215 consoles > but support 3270 consoles if found. > > --- > src/core/manager.c | 24 ++++++++++++---- > src/shared/util.c | 80 > +++++++++++++++++++++++++++++++++++++++++++++++++--- > src/shared/util.h | 1 + > 3 files changed, 96 insertions(+), 9 deletions(-) > > diff --git src/core/manager.c src/core/manager.c > index 0cb2044..f78ac18 100644 > --- src/core/manager.c > +++ src/core/manager.c > @@ -112,7 +112,7 @@ static int manager_watch_jobs_in_progress(Manager *m) { > > #define CYLON_BUFFER_EXTRA (2*(sizeof(ANSI_RED_ON)-1) + > sizeof(ANSI_HIGHLIGHT_RED_ON)-1 + 2*(sizeof(ANSI_HIGHLIGHT_OFF)-1))
The goal of this patch seems correct. Nevertheless, it is quite intrusive. Could you rework the patch to: 1. centralize the ansi console detection, like we do with on_tty(), so it is implicitly checked on first access and then cached inside of the function; 2. replace ANSI_RED_ON/OFF/... with ansi_red_on(), which calls the ansi console status function and returns either ANSI_RED_ON/OFF/... or ""; 3. instead of parsing the message to extract status in the status_printf function, pass the status as a (numerical) argument, and use this to decide the output. The mapping should be stored in a table; 4. use parse_proc_cmdline to parse /proc/cmdline. Thanks, Zbyszek > > -static void draw_cylon(char buffer[], size_t buflen, unsigned width, > unsigned pos) { > +static void draw_cylon(char buffer[], size_t buflen, unsigned width, > unsigned pos, bool ansi_console) { > char *p = buffer; > > assert(buflen >= CYLON_BUFFER_EXTRA + width + 1); > @@ -121,12 +121,14 @@ static void draw_cylon(char buffer[], size_t buflen, > unsigned width, unsigned po > if (pos > 1) { > if (pos > 2) > p = mempset(p, ' ', pos-2); > - p = stpcpy(p, ANSI_RED_ON); > + if (ansi_console) > + p = stpcpy(p, ANSI_RED_ON); > *p++ = '*'; > } > > if (pos > 0 && pos <= width) { > - p = stpcpy(p, ANSI_HIGHLIGHT_RED_ON); > + if (ansi_console) > + p = stpcpy(p, ANSI_HIGHLIGHT_RED_ON); > *p++ = '*'; > } > > @@ -137,7 +139,8 @@ static void draw_cylon(char buffer[], size_t buflen, > unsigned width, unsigned po > *p++ = '*'; > if (pos < width-1) > p = mempset(p, ' ', width-1-pos); > - strcpy(p, ANSI_HIGHLIGHT_OFF); > + if (ansi_console) > + strcpy(p, ANSI_HIGHLIGHT_OFF); > } > } > > @@ -154,6 +157,7 @@ void manager_flip_auto_status(Manager *m, bool enable) { > } > > static void manager_print_jobs_in_progress(Manager *m) { > + static int is_ansi_console = -1; > _cleanup_free_ char *job_of_n = NULL; > Iterator i; > Job *j; > @@ -178,10 +182,20 @@ static void manager_print_jobs_in_progress(Manager *m) { > assert(counter == print_nr + 1); > assert(j); > > + if (_unlikely_(is_ansi_console < 0)) { > + int fd = open_terminal("/dev/console", > O_RDONLY|O_NOCTTY|O_CLOEXEC); > + if (fd < 0) > + is_ansi_console = 0; > + else { > + is_ansi_console = (int)ansi_console(fd); > + close(fd); > + } > + } > + > cylon_pos = m->jobs_in_progress_iteration % 14; > if (cylon_pos >= 8) > cylon_pos = 14 - cylon_pos; > - draw_cylon(cylon, sizeof(cylon), 6, cylon_pos); > + draw_cylon(cylon, sizeof(cylon), 6, cylon_pos, > (bool)is_ansi_console); > > m->jobs_in_progress_iteration++; > > diff --git src/shared/util.c src/shared/util.c > index a7aec5c..03b01de 100644 > --- src/shared/util.c > +++ src/shared/util.c > @@ -2937,6 +2937,7 @@ int status_vprintf(const char *status, bool ellipse, > bool ephemeral, const char > struct iovec iovec[6] = {}; > int n = 0; > static bool prev_ephemeral; > + static int is_ansi_console = -1; > > assert(format); > > @@ -2950,6 +2951,41 @@ int status_vprintf(const char *status, bool ellipse, > bool ephemeral, const char > if (fd < 0) > return fd; > > + if (_unlikely_(is_ansi_console < 0)) > + is_ansi_console = (int)ansi_console(fd); > + > + if (status && !is_ansi_console) { > + const char *esc, *ptr; > + esc = strchr(status, 0x1B); > + if (esc && (ptr = strpbrk(esc, "SOFDTI*"))) { > + switch(*ptr) { > + case 'S': > + status = " SKIP "; > + break; > + case 'O': > + status = " OK "; > + break; > + case 'F': > + status = "FAILED"; > + break; > + case 'D': > + status = "DEPEND"; > + break; > + case 'T': > + status = " TIME "; > + break; > + case 'I': > + status = " INFO "; > + break; > + case '*': > + status = " BUSY "; > + break; > + default: > + break; > + } > + } > + } > + > if (ellipse) { > char *e; > size_t emax, sl; > @@ -2972,8 +3008,12 @@ int status_vprintf(const char *status, bool ellipse, > bool ephemeral, const char > } > } > > - if (prev_ephemeral) > - IOVEC_SET_STRING(iovec[n++], "\r" ANSI_ERASE_TO_END_OF_LINE); > + if (prev_ephemeral) { > + if (is_ansi_console) > + IOVEC_SET_STRING(iovec[n++], "\r" > ANSI_ERASE_TO_END_OF_LINE); > + else > + IOVEC_SET_STRING(iovec[n++], "\r"); > + } > prev_ephemeral = ephemeral; > > if (status) { > @@ -3220,8 +3260,22 @@ void columns_lines_cache_reset(int signum) { > bool on_tty(void) { > static int cached_on_tty = -1; > > - if (_unlikely_(cached_on_tty < 0)) > + if (_unlikely_(cached_on_tty < 0)) { > cached_on_tty = isatty(STDOUT_FILENO) > 0; > +#if defined (__s390__) || defined (__s390x__) > + if (cached_on_tty) { > + const char *e = getenv("TERM"); > + if (!e) > + return cached_on_tty; > + if (streq(e, "dumb") || strneq(e, "ibm3", 4)) { > + char *mode = NULL; > + int r = parse_env_file("/proc/cmdline", > WHITESPACE, "conmode", &mode, NULL); > + if (r < 0 || !mode || !streq(mode, "3270")) > + cached_on_tty = 0; > + } > + } > +#endif > + } > > return cached_on_tty; > } > @@ -3711,7 +3765,25 @@ bool tty_is_vc_resolve(const char *tty) { > const char *default_term_for_tty(const char *tty) { > assert(tty); > > - return tty_is_vc_resolve(tty) ? "TERM=linux" : "TERM=vt102"; > + if (tty_is_vc_resolve(tty)) > + return "TERM=linux"; > + > + if (startswith(tty, "/dev/")) > + tty += 5; > + > +#if defined (__s390__) || defined (__s390x__) > + if (streq(tty, "ttyS0")) { > + char *mode = NULL; > + int r = parse_env_file("/proc/cmdline", WHITESPACE, > "conmode", &mode, NULL); > + if (r < 0 || !mode || !streq(mode, "3270")) > + return "TERM=dumb"; > + if (streq(mode, "3270")) > + return "TERM=ibm327x"; > + } > + if (streq(tty, "ttyS1")) > + return "TERM=vt220"; > +#endif > + return "TERM=vt102"; > } > > bool dirent_is_file(const struct dirent *de) { > diff --git src/shared/util.h src/shared/util.h > index 1796014..2dc918d 100644 > --- src/shared/util.h > +++ src/shared/util.h > @@ -446,6 +446,7 @@ unsigned lines(void); > void columns_lines_cache_reset(int _unused_ signum); > > bool on_tty(void); > +bool ansi_console(int fd); > > static inline const char *ansi_highlight(void) { > return on_tty() ? ANSI_HIGHLIGHT_ON : ""; > -- > 1.7.9.2 > > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel > _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel