[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Jun 9 01:04:03 PDT 2020

labath added a comment.

I think this looks good now. All that's now left is a test case. I think it should be possible to test this in a number of ways:

- CommandReturnObject unittest
- a shell test which forces `use-color` to true
- a python test which forces `use-color` to true
- a pexpect test

Comment at: lldb/include/lldb/Utility/Stream.h:402-403
+    bool has_colors() const override { return true; }
     uint64_t current_pos() const override {
This is not consistent with the intend of this method (`This function determines if this stream is displayed and supports colors.`). One could argue whether this could be "true" for CommandObjectResult streams which are destined to be later printed to a terminal, but it's definitely not a good default.

However, all of that is irrelevant, as this should not be needed anymore. Is that correct?

Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:212
-  CommandReturnObject result;
+  CommandReturnObject result(m_debugger.GetUseColor());
I'm wondering in how many places do we construct `CommandReturnObject`s. If it's not too many, it may make sense to remove the `=false` from the colors argument, to force the user to think about the right value...



More information about the lldb-commits mailing list