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

[Thomas Huth]

On 24/10/2020 13.24, Christian Schoenebeck wrote:
> On Samstag, 24. Oktober 2020 08:04:20 CEST Thomas Huth wrote:
>> On 08/10/2020 20.34, Christian Schoenebeck wrote:
>>> This new function is purely for debugging purposes. It prints the
>>> current qos graph to stdout and allows to identify problems in the
>>> created qos graph e.g. when writing new qos tests.
>>>
>>> Coloured output is used to mark available nodes in green colour,
>>> whereas unavailable nodes are marked in red colour.
>>>
>>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>>> ---
>>>
>>>  tests/qtest/libqos/qgraph.c | 56 +++++++++++++++++++++++++++++++++++++
>>>  tests/qtest/libqos/qgraph.h | 20 +++++++++++++
>>>  2 files changed, 76 insertions(+)
>>>
>>> diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c
>>> index 61faf6b27d..af93e38dcb 100644
>>> --- a/tests/qtest/libqos/qgraph.c
>>> +++ b/tests/qtest/libqos/qgraph.c
>>> @@ -805,3 +805,59 @@ void qos_delete_cmd_line(const char *name)
>>>
>>>          node->command_line = NULL;
>>>      
>>>      }
>>>  
>>>  }
>>>
>>> +
>>> +#define RED(txt) (    \
>>> +    "\033[0;91m" txt  \
>>> +    "\033[0m"         \
>>> +)
>>> +
>>> +#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?
>>
>>  Thomas
> 
> The precise way I did it here is definitely unclean. And the use case is 
> trivial, so on doubt I could just drop it of course.
> 
> But allow me one attempt to promote coloured terminal output in general: 
> These 
> are ANSI color escape sequences, a standard with its youngest revision dating 
> back to 1991. It is a well supported standard on all major platforms nowadays:
> 
>       https://en.wikipedia.org/wiki/ANSI_escape_code
> 
> It works on macOS's standard terminal for at least 20 years, with cmd.exe on 
> Windows 10, on essentially all Linux and BSD distros, and even on many web 
> based CI platforms.
> 
> So what about introducing some globally shared macros for coloured output 
> instead? Then there would be one central place for changing coloured output 
> support for the entire code base; and I would change the macros to fallback 
> to 
> plain text output if there is any doubt the terminal would not support it.
> 
> Besides, QEMU just switched to meson which uses coloured output as well, as 
> do 
> clang, GCC, git and many other tools in your build chain.

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.

 Thomas