[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 04/12] libqos/qgraph: add qos_dump_graph()
From: |
Christian Schoenebeck |
Subject: |
Re: [PATCH v4 04/12] libqos/qgraph: add qos_dump_graph() |
Date: |
Wed, 28 Oct 2020 14:28:10 +0100 |
On Mittwoch, 28. Oktober 2020 14:00:21 CET Eric Blake wrote:
> On 10/28/20 12:51 AM, Thomas Huth wrote:
> >>>> +#define GREEN(txt) ( \
> >>>> + "\033[0;92m" txt \
> >>>> + "\033[0m" \
> >>>> +)
> >>>
> >>> I don't think this is very portable - and it will only make logs ugly to
> >>> read in text editors. Could you please simply drop these macros?
> >
> > Sure, colored output is nice, but we certainly also need a way to disable
> > it, e.g. if you want to collect the log in a file and then have a look at
> > it in a text editor.
>
> Agreed. GNU libtextstyle
> (https://www.gnu.org/software/gettext/libtextstyle/manual/libtextstyle.html)
> is a much more portable way to do colored output where it becomes easy to
> enable/disable or even adjust the colors to user preferences. Sadly, it is
> GPLv3+, and thus unusable for qemu. But the bare minimum that you must
> have when making colored output gated on whether stdout is a
> terminal (that is, any program that does color should have a
> --color=off|auto|on command-line option, and that in turn implies
> function calls rather than macros to properly encapsulate the decision
> logic.
Not sure if it would make sense adding another dependency just for colour
support in QEMU anyway, because rendering the right output sequence is not the
big issue, nor auto detecting tty colour support, nor handling user configs. A
large number of apps already do that in-tree / inline.
The challenge in QEMU though (in contrast to stand-alone apps) is integrating
this meaningful for all the (quite different) output channels in QEMU, e.g.
host logs, test case output, different modes, etc., while catching misusage
and retaining a simple API.
I postpone the colour issue for that reason and drop colour from these patches
for now. I'll probably rather come up with a dedicated series attempt just for
colour at some later point.
Best regards,
Christian Schoenebeck
- Re: [PATCH v4 01/12] libqos/qgraph: add qemu_name to QOSGraphNode, (continued)
[PATCH v4 02/12] libqos/qgraph: add qos_node_create_driver_named(), Christian Schoenebeck, 2020/10/08
[PATCH v4 03/12] libqos/qgraph_internal: add qos_printf() and qos_printf_literal(), Christian Schoenebeck, 2020/10/08
[PATCH v4 04/12] libqos/qgraph: add qos_dump_graph(), Christian Schoenebeck, 2020/10/08
[PATCH v4 05/12] tests/qtest/qos-test: dump qos graph if verbose, Christian Schoenebeck, 2020/10/08
[PATCH v4 06/12] tests/qtest/qos-test: dump environment variables if verbose, Christian Schoenebeck, 2020/10/08
[PATCH v4 07/12] tests/qtest/qos-test: dump QEMU command if verbose, Christian Schoenebeck, 2020/10/08
[PATCH v4 08/12] tests/9pfs: change qtest name prefix to synth, Christian Schoenebeck, 2020/10/08