| Message ID | 20210316221648.11839-1-sonnysasaka@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | [BlueZ] monitor: Add option to force output color | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=449559 ---Test result--- ############################## Test: CheckPatch - PASS ############################## Test: CheckGitLint - PASS ############################## Test: CheckBuild - PASS ############################## Test: MakeCheck - PASS --- Regards, Linux Bluetooth
Hi Luiz/Marcel, Friendly ping to review this patch. Thanks! On Tue, Mar 16, 2021 at 3:17 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > Sometimes we want to force output color even when stdout is not a > terminal, for example when piping the output to a filter script and then > piping it further to a pager which can display colors. > > This patch provides a general option to force whether color is on or off > (always and never), or leave btmon to decide (auto). > > Reviewed-by: Daniel Winkler <danielwinkler@google.com> > > --- > monitor/display.c | 12 +++++++++++- > monitor/display.h | 3 +++ > monitor/main.c | 17 ++++++++++++++++- > 3 files changed, 30 insertions(+), 2 deletions(-) > > diff --git a/monitor/display.c b/monitor/display.c > index 4e5693b04..d61a79a38 100644 > --- a/monitor/display.c > +++ b/monitor/display.c > @@ -29,12 +29,22 @@ > > static pid_t pager_pid = 0; > int default_pager_num_columns = FALLBACK_TERMINAL_WIDTH; > +enum monitor_color setting_monitor_color = COLOR_AUTO; > + > +void set_monitor_color(enum monitor_color color) > +{ > + setting_monitor_color = color; > +} > > bool use_color(void) > { > static int cached_use_color = -1; > > - if (__builtin_expect(!!(cached_use_color < 0), 0)) > + if (setting_monitor_color == COLOR_ALWAYS) > + cached_use_color = 1; > + else if (setting_monitor_color == COLOR_NEVER) > + cached_use_color = 0; > + else if (__builtin_expect(!!(cached_use_color < 0), 0)) > cached_use_color = isatty(STDOUT_FILENO) > 0 || pager_pid > 0; > > return cached_use_color; > diff --git a/monitor/display.h b/monitor/display.h > index cba39ec7f..be5739833 100644 > --- a/monitor/display.h > +++ b/monitor/display.h > @@ -14,6 +14,9 @@ > > bool use_color(void); > > +enum monitor_color { COLOR_AUTO, COLOR_ALWAYS, COLOR_NEVER }; > +void set_monitor_color(enum monitor_color); > + > #define COLOR_OFF "\x1B[0m" > #define COLOR_BLACK "\x1B[0;30m" > #define COLOR_RED "\x1B[0;31m" > diff --git a/monitor/main.c b/monitor/main.c > index 969c88103..3ec3a5f08 100644 > --- a/monitor/main.c > +++ b/monitor/main.c > @@ -69,6 +69,7 @@ static void usage(void) > "\t-R --rtt [<address>],[<area>],[<name>]\n" > "\t RTT control block parameters\n" > "\t-C, --columns [width] Output width if not a terminal\n" > + "\t-c, --color [mode] Output color: auto/always/never\n" > "\t-h, --help Show help options\n"); > } > > @@ -93,6 +94,7 @@ static const struct option main_options[] = { > { "jlink", required_argument, NULL, 'J' }, > { "rtt", required_argument, NULL, 'R' }, > { "columns", required_argument, NULL, 'C' }, > + { "color", required_argument, NULL, 'c' }, > { "todo", no_argument, NULL, '#' }, > { "version", no_argument, NULL, 'v' }, > { "help", no_argument, NULL, 'h' }, > @@ -124,7 +126,7 @@ int main(int argc, char *argv[]) > struct sockaddr_un addr; > > opt = getopt_long(argc, argv, > - "r:w:a:s:p:i:d:B:V:MNtTSAE:PJ:R:C:vh", > + "r:w:a:s:p:i:d:B:V:MNtTSAE:PJ:R:C:c:vh", > main_options, NULL); > if (opt < 0) > break; > @@ -211,6 +213,19 @@ int main(int argc, char *argv[]) > case 'C': > set_default_pager_num_columns(atoi(optarg)); > break; > + case 'c': > + if (strcmp("always", optarg) == 0) > + set_monitor_color(COLOR_ALWAYS); > + else if (strcmp("never", optarg) == 0) > + set_monitor_color(COLOR_NEVER); > + else if (strcmp("auto", optarg) == 0) > + set_monitor_color(COLOR_AUTO); > + else { > + fprintf(stderr, "Color option must be one of " > + "auto/always/never\n"); > + return EXIT_FAILURE; > + } > + break; > case '#': > packet_todo(); > lmp_todo(); > -- > 2.29.2 >
Hi Sonny, On Thu, Mar 18, 2021 at 11:19 AM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > Hi Luiz/Marcel, > > Friendly ping to review this patch. Thanks! > > > On Tue, Mar 16, 2021 at 3:17 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > > > Sometimes we want to force output color even when stdout is not a > > terminal, for example when piping the output to a filter script and then > > piping it further to a pager which can display colors. > > > > This patch provides a general option to force whether color is on or off > > (always and never), or leave btmon to decide (auto). > > > > Reviewed-by: Daniel Winkler <danielwinkler@google.com> > > > > --- > > monitor/display.c | 12 +++++++++++- > > monitor/display.h | 3 +++ > > monitor/main.c | 17 ++++++++++++++++- > > 3 files changed, 30 insertions(+), 2 deletions(-) > > > > diff --git a/monitor/display.c b/monitor/display.c > > index 4e5693b04..d61a79a38 100644 > > --- a/monitor/display.c > > +++ b/monitor/display.c > > @@ -29,12 +29,22 @@ > > > > static pid_t pager_pid = 0; > > int default_pager_num_columns = FALLBACK_TERMINAL_WIDTH; > > +enum monitor_color setting_monitor_color = COLOR_AUTO; > > + > > +void set_monitor_color(enum monitor_color color) > > +{ > > + setting_monitor_color = color; > > +} > > > > bool use_color(void) > > { > > static int cached_use_color = -1; > > > > - if (__builtin_expect(!!(cached_use_color < 0), 0)) > > + if (setting_monitor_color == COLOR_ALWAYS) > > + cached_use_color = 1; > > + else if (setting_monitor_color == COLOR_NEVER) > > + cached_use_color = 0; > > + else if (__builtin_expect(!!(cached_use_color < 0), 0)) > > cached_use_color = isatty(STDOUT_FILENO) > 0 || pager_pid > 0; > > > > return cached_use_color; > > diff --git a/monitor/display.h b/monitor/display.h > > index cba39ec7f..be5739833 100644 > > --- a/monitor/display.h > > +++ b/monitor/display.h > > @@ -14,6 +14,9 @@ > > > > bool use_color(void); > > > > +enum monitor_color { COLOR_AUTO, COLOR_ALWAYS, COLOR_NEVER }; > > +void set_monitor_color(enum monitor_color); > > + > > #define COLOR_OFF "\x1B[0m" > > #define COLOR_BLACK "\x1B[0;30m" > > #define COLOR_RED "\x1B[0;31m" > > diff --git a/monitor/main.c b/monitor/main.c > > index 969c88103..3ec3a5f08 100644 > > --- a/monitor/main.c > > +++ b/monitor/main.c > > @@ -69,6 +69,7 @@ static void usage(void) > > "\t-R --rtt [<address>],[<area>],[<name>]\n" > > "\t RTT control block parameters\n" > > "\t-C, --columns [width] Output width if not a terminal\n" > > + "\t-c, --color [mode] Output color: auto/always/never\n" > > "\t-h, --help Show help options\n"); > > } > > > > @@ -93,6 +94,7 @@ static const struct option main_options[] = { > > { "jlink", required_argument, NULL, 'J' }, > > { "rtt", required_argument, NULL, 'R' }, > > { "columns", required_argument, NULL, 'C' }, > > + { "color", required_argument, NULL, 'c' }, > > { "todo", no_argument, NULL, '#' }, > > { "version", no_argument, NULL, 'v' }, > > { "help", no_argument, NULL, 'h' }, > > @@ -124,7 +126,7 @@ int main(int argc, char *argv[]) > > struct sockaddr_un addr; > > > > opt = getopt_long(argc, argv, > > - "r:w:a:s:p:i:d:B:V:MNtTSAE:PJ:R:C:vh", > > + "r:w:a:s:p:i:d:B:V:MNtTSAE:PJ:R:C:c:vh", > > main_options, NULL); > > if (opt < 0) > > break; > > @@ -211,6 +213,19 @@ int main(int argc, char *argv[]) > > case 'C': > > set_default_pager_num_columns(atoi(optarg)); > > break; > > + case 'c': > > + if (strcmp("always", optarg) == 0) > > + set_monitor_color(COLOR_ALWAYS); > > + else if (strcmp("never", optarg) == 0) > > + set_monitor_color(COLOR_NEVER); > > + else if (strcmp("auto", optarg) == 0) > > + set_monitor_color(COLOR_AUTO); > > + else { > > + fprintf(stderr, "Color option must be one of " > > + "auto/always/never\n"); > > + return EXIT_FAILURE; > > + } > > + break; > > case '#': > > packet_todo(); > > lmp_todo(); > > -- > > 2.29.2 > > Applied, thanks.
diff --git a/monitor/display.c b/monitor/display.c index 4e5693b04..d61a79a38 100644 --- a/monitor/display.c +++ b/monitor/display.c @@ -29,12 +29,22 @@ static pid_t pager_pid = 0; int default_pager_num_columns = FALLBACK_TERMINAL_WIDTH; +enum monitor_color setting_monitor_color = COLOR_AUTO; + +void set_monitor_color(enum monitor_color color) +{ + setting_monitor_color = color; +} bool use_color(void) { static int cached_use_color = -1; - if (__builtin_expect(!!(cached_use_color < 0), 0)) + if (setting_monitor_color == COLOR_ALWAYS) + cached_use_color = 1; + else if (setting_monitor_color == COLOR_NEVER) + cached_use_color = 0; + else if (__builtin_expect(!!(cached_use_color < 0), 0)) cached_use_color = isatty(STDOUT_FILENO) > 0 || pager_pid > 0; return cached_use_color; diff --git a/monitor/display.h b/monitor/display.h index cba39ec7f..be5739833 100644 --- a/monitor/display.h +++ b/monitor/display.h @@ -14,6 +14,9 @@ bool use_color(void); +enum monitor_color { COLOR_AUTO, COLOR_ALWAYS, COLOR_NEVER }; +void set_monitor_color(enum monitor_color); + #define COLOR_OFF "\x1B[0m" #define COLOR_BLACK "\x1B[0;30m" #define COLOR_RED "\x1B[0;31m" diff --git a/monitor/main.c b/monitor/main.c index 969c88103..3ec3a5f08 100644 --- a/monitor/main.c +++ b/monitor/main.c @@ -69,6 +69,7 @@ static void usage(void) "\t-R --rtt [<address>],[<area>],[<name>]\n" "\t RTT control block parameters\n" "\t-C, --columns [width] Output width if not a terminal\n" + "\t-c, --color [mode] Output color: auto/always/never\n" "\t-h, --help Show help options\n"); } @@ -93,6 +94,7 @@ static const struct option main_options[] = { { "jlink", required_argument, NULL, 'J' }, { "rtt", required_argument, NULL, 'R' }, { "columns", required_argument, NULL, 'C' }, + { "color", required_argument, NULL, 'c' }, { "todo", no_argument, NULL, '#' }, { "version", no_argument, NULL, 'v' }, { "help", no_argument, NULL, 'h' }, @@ -124,7 +126,7 @@ int main(int argc, char *argv[]) struct sockaddr_un addr; opt = getopt_long(argc, argv, - "r:w:a:s:p:i:d:B:V:MNtTSAE:PJ:R:C:vh", + "r:w:a:s:p:i:d:B:V:MNtTSAE:PJ:R:C:c:vh", main_options, NULL); if (opt < 0) break; @@ -211,6 +213,19 @@ int main(int argc, char *argv[]) case 'C': set_default_pager_num_columns(atoi(optarg)); break; + case 'c': + if (strcmp("always", optarg) == 0) + set_monitor_color(COLOR_ALWAYS); + else if (strcmp("never", optarg) == 0) + set_monitor_color(COLOR_NEVER); + else if (strcmp("auto", optarg) == 0) + set_monitor_color(COLOR_AUTO); + else { + fprintf(stderr, "Color option must be one of " + "auto/always/never\n"); + return EXIT_FAILURE; + } + break; case '#': packet_todo(); lmp_todo();
Sometimes we want to force output color even when stdout is not a terminal, for example when piping the output to a filter script and then piping it further to a pager which can display colors. This patch provides a general option to force whether color is on or off (always and never), or leave btmon to decide (auto). Reviewed-by: Daniel Winkler <danielwinkler@google.com> --- monitor/display.c | 12 +++++++++++- monitor/display.h | 3 +++ monitor/main.c | 17 ++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-)