2021-03-16 22:59:56

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH BlueZ] monitor: Add option to force output color

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 <[email protected]>

---
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


2021-03-18 18:19:46

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH BlueZ] monitor: Add option to force output color

Hi Luiz/Marcel,

Friendly ping to review this patch. Thanks!


On Tue, Mar 16, 2021 at 3:17 PM Sonny Sasaka <[email protected]> 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 <[email protected]>
>
> ---
> 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
>

2021-03-18 23:03:57

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ] monitor: Add option to force output color

Hi Sonny,

On Thu, Mar 18, 2021 at 11:19 AM Sonny Sasaka <[email protected]> wrote:
>
> Hi Luiz/Marcel,
>
> Friendly ping to review this patch. Thanks!
>
>
> On Tue, Mar 16, 2021 at 3:17 PM Sonny Sasaka <[email protected]> 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 <[email protected]>
> >
> > ---
> > 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.

--
Luiz Augusto von Dentz