Re: [PATCH v4 04/12] libqos/qgraph: add qos_dump_graph()

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

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