[lldb/Interpreter] Support color in CommandReturnObject

JDevlieghere · JDevlieghere · commit de019b88dd58 · 2020-06-09T10:45:45.000-07:00
Color the error: and warning: part of the CommandReturnObject output, similar to how an error is printed from the driver when colors are enabled. Differential revision: https://reviews.llvm.org/D81058
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -16,14 +16,15 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/WithColor.h"
 
 #include <memory>
 
 namespace lldb_private {
 
 class CommandReturnObject {
 public:
-  CommandReturnObject();
+  CommandReturnObject(bool colors);
 
   ~CommandReturnObject();
 
diff --git a/lldb/include/lldb/Utility/Stream.h b/lldb/include/lldb/Utility/Stream.h
@@ -56,12 +56,13 @@ class Stream {
   ///
   /// Construct with dump flags \a flags and the default address size. \a
   /// flags can be any of the above enumeration logical OR'ed together.
-  Stream(uint32_t flags, uint32_t addr_size, lldb::ByteOrder byte_order);
+  Stream(uint32_t flags, uint32_t addr_size, lldb::ByteOrder byte_order,
+         bool colors = false);
 
   /// Construct a default Stream, not binary, host byte order and host addr
   /// size.
   ///
-  Stream();
+  Stream(bool colors = false);
 
   // FIXME: Streams should not be copyable.
   Stream(const Stream &other) : m_forwarder(*this) { (*this) = other; }
@@ -403,8 +404,10 @@ class Stream {
     }
 
   public:
-    RawOstreamForward(Stream &target)
-        : llvm::raw_ostream(/*unbuffered*/ true), m_target(target) {}
+    RawOstreamForward(Stream &target, bool colors = false)
+        : llvm::raw_ostream(/*unbuffered*/ true), m_target(target) {
+      enable_colors(colors);
+    }
   };
   RawOstreamForward m_forwarder;
 };
diff --git a/lldb/include/lldb/Utility/StreamTee.h b/lldb/include/lldb/Utility/StreamTee.h
@@ -19,7 +19,8 @@ namespace lldb_private {
 
 class StreamTee : public Stream {
 public:
-  StreamTee() : Stream(), m_streams_mutex(), m_streams() {}
+  StreamTee(bool colors = false)
+      : Stream(colors), m_streams_mutex(), m_streams() {}
 
   StreamTee(lldb::StreamSP &stream_sp)
       : Stream(), m_streams_mutex(), m_streams() {
diff --git a/lldb/source/API/SBCommandReturnObject.cpp b/lldb/source/API/SBCommandReturnObject.cpp
@@ -22,7 +22,7 @@ using namespace lldb_private;
 class lldb_private::SBCommandReturnObjectImpl {
 public:
   SBCommandReturnObjectImpl()
-      : m_ptr(new CommandReturnObject()), m_owned(true) {}
+      : m_ptr(new CommandReturnObject(false)), m_owned(true) {}
   SBCommandReturnObjectImpl(CommandReturnObject &ref)
       : m_ptr(&ref), m_owned(false) {}
   SBCommandReturnObjectImpl(const SBCommandReturnObjectImpl &rhs)
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -630,11 +630,11 @@ bool BreakpointOptions::BreakpointOptionsCallbackFunction(
     ExecutionContext exe_ctx(context->exe_ctx_ref);
     Target *target = exe_ctx.GetTargetPtr();
     if (target) {
-      CommandReturnObject result;
       Debugger &debugger = target->GetDebugger();
+      CommandReturnObject result(debugger.GetUseColor());
+
       // Rig up the results secondary output stream to the debugger's, so the
       // output will come out synchronously if the debugger is set up that way.
-
       StreamSP output_stream(debugger.GetAsyncOutputStream());
       StreamSP error_stream(debugger.GetAsyncErrorStream());
       result.SetImmediateOutputStream(output_stream);
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -500,7 +500,8 @@ void CommandObjectExpression::IOHandlerInputComplete(IOHandler &io_handler,
   StreamFileSP output_sp = io_handler.GetOutputStreamFileSP();
   StreamFileSP error_sp = io_handler.GetErrorStreamFileSP();
 
-  CommandReturnObject return_obj;
+  CommandReturnObject return_obj(
+      GetCommandInterpreter().GetDebugger().GetUseColor());
   EvaluateExpression(line.c_str(), *output_sp, *error_sp, return_obj);
   if (output_sp)
     output_sp->Flush();
diff --git a/lldb/source/Commands/CommandObjectWatchpointCommand.cpp b/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
@@ -282,12 +282,12 @@ are no syntax errors may indicate that a function was declared but never called.
       ExecutionContext exe_ctx(context->exe_ctx_ref);
       Target *target = exe_ctx.GetTargetPtr();
       if (target) {
-        CommandReturnObject result;
         Debugger &debugger = target->GetDebugger();
+        CommandReturnObject result(debugger.GetUseColor());
+
         // Rig up the results secondary output stream to the debugger's, so the
         // output will come out synchronously if the debugger is set up that
         // way.
-
         StreamSP output_stream(debugger.GetAsyncOutputStream());
         StreamSP error_stream(debugger.GetAsyncErrorStream());
         result.SetImmediateOutputStream(output_stream);
diff --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp
@@ -216,7 +216,7 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) {
           ci.SetPromptOnQuit(false);
 
         // Execute the command
-        CommandReturnObject result;
+        CommandReturnObject result(debugger.GetUseColor());
         result.SetImmediateOutputStream(output_sp);
         result.SetImmediateErrorStream(error_sp);
         ci.HandleCommand(code.c_str(), eLazyBoolNo, result);
diff --git a/lldb/source/Interpreter/CommandAlias.cpp b/lldb/source/Interpreter/CommandAlias.cpp
@@ -32,7 +32,7 @@ static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP &cmd_obj_sp,
   std::string options_string(options_args);
   // TODO: Find a way to propagate errors in this CommandReturnObject up the
   // stack.
-  CommandReturnObject result;
+  CommandReturnObject result(false);
   // Check to see if the command being aliased can take any command options.
   Options *options = cmd_obj_sp->GetOptions();
   if (options) {
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -209,7 +209,7 @@ void CommandInterpreter::Initialize() {
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
 
-  CommandReturnObject result;
+  CommandReturnObject result(m_debugger.GetUseColor());
 
   LoadCommandDictionary();
 
@@ -2264,7 +2264,7 @@ void CommandInterpreter::HandleCommands(const StringList &commands,
                                      m_debugger.GetPrompt().str().c_str(), cmd);
     }
 
-    CommandReturnObject tmp_result;
+    CommandReturnObject tmp_result(m_debugger.GetUseColor());
     // If override_context is not NULL, pass no_context_switching = true for
     // HandleCommand() since we updated our context already.
 
@@ -2792,7 +2792,7 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
 
   StartHandlingCommand();
 
-  lldb_private::CommandReturnObject result;
+  lldb_private::CommandReturnObject result(m_debugger.GetUseColor());
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
   // Now emit the command output text from the command we just executed
diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp
@@ -282,7 +282,7 @@ void CommandObject::HandleCompletion(CompletionRequest &request) {
   } else {
     // Can we do anything generic with the options?
     Options *cur_options = GetOptions();
-    CommandReturnObject result;
+    CommandReturnObject result(m_interpreter.GetDebugger().GetUseColor());
     OptionElementVector opt_element_vector;
 
     if (cur_options != nullptr) {
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -14,6 +14,18 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static llvm::raw_ostream &error(Stream &strm) {
+  return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Error,
+                         llvm::ColorMode::Enable)
+         << "error: ";
+}
+
+static llvm::raw_ostream &warning(Stream &strm) {
+  return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Warning,
+                         llvm::ColorMode::Enable)
+         << "warning: ";
+}
+
 static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
   bool add_newline = false;
   if (!s.empty()) {
@@ -28,9 +40,10 @@ static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
     strm.EOL();
 }
 
-CommandReturnObject::CommandReturnObject()
-    : m_out_stream(), m_err_stream(), m_status(eReturnStatusStarted),
-      m_did_change_process_state(false), m_interactive(true) {}
+CommandReturnObject::CommandReturnObject(bool colors)
+    : m_out_stream(colors), m_err_stream(colors),
+      m_status(eReturnStatusStarted), m_did_change_process_state(false),
+      m_interactive(true) {}
 
 CommandReturnObject::~CommandReturnObject() {}
 
@@ -45,9 +58,8 @@ void CommandReturnObject::AppendErrorWithFormat(const char *format, ...) {
 
   const std::string &s = std::string(sstrm.GetString());
   if (!s.empty()) {
-    Stream &error_strm = GetErrorStream();
-    error_strm.PutCString("error: ");
-    DumpStringToStreamWithNewline(error_strm, s);
+    error(GetErrorStream());
+    DumpStringToStreamWithNewline(GetErrorStream(), s);
   }
 }
 
@@ -72,7 +84,7 @@ void CommandReturnObject::AppendWarningWithFormat(const char *format, ...) {
   sstrm.PrintfVarArg(format, args);
   va_end(args);
 
-  GetErrorStream() << "warning: " << sstrm.GetString();
+  warning(GetErrorStream()) << sstrm.GetString();
 }
 
 void CommandReturnObject::AppendMessage(llvm::StringRef in_string) {
@@ -84,7 +96,7 @@ void CommandReturnObject::AppendMessage(llvm::StringRef in_string) {
 void CommandReturnObject::AppendWarning(llvm::StringRef in_string) {
   if (in_string.empty())
     return;
-  GetErrorStream() << "warning: " << in_string << "\n";
+  warning(GetErrorStream()) << in_string << '\n';
 }
 
 // Similar to AppendWarning, but do not prepend 'warning: ' to message, and
@@ -99,7 +111,7 @@ void CommandReturnObject::AppendRawWarning(llvm::StringRef in_string) {
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   if (in_string.empty())
     return;
-  GetErrorStream() << "error: " << in_string << "\n";
+  error(GetErrorStream()) << in_string << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
diff --git a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -981,7 +981,7 @@ EnableOptionsSP ParseAutoEnableOptions(Status &error, Debugger &debugger) {
   EnableOptionsSP options_sp(new EnableOptions());
   options_sp->NotifyOptionParsingStarting(&exe_ctx);
 
-  CommandReturnObject result;
+  CommandReturnObject result(debugger.GetUseColor());
 
   // Parse the arguments.
   auto options_property_sp =
@@ -1036,7 +1036,7 @@ bool RunEnableCommand(CommandInterpreter &interpreter) {
   }
 
   // Run the command.
-  CommandReturnObject return_object;
+  CommandReturnObject return_object(interpreter.GetDebugger().GetUseColor());
   interpreter.HandleCommand(command_stream.GetData(), eLazyBoolNo,
                             return_object);
   return return_object.Succeeded();
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
@@ -2556,7 +2556,7 @@ void Target::RunStopHooks() {
   if (!any_active_hooks)
     return;
 
-  CommandReturnObject result;
+  CommandReturnObject result(m_debugger.GetUseColor());
 
   std::vector<ExecutionContext> exc_ctx_with_reasons;
   std::vector<SymbolContext> sym_ctx_with_reasons;
diff --git a/lldb/source/Utility/Stream.cpp b/lldb/source/Utility/Stream.cpp
@@ -22,13 +22,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order)
+Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order,
+               bool colors)
     : m_flags(flags), m_addr_size(addr_size), m_byte_order(byte_order),
-      m_indent_level(0), m_forwarder(*this) {}
+      m_indent_level(0), m_forwarder(*this, colors) {}
 
-Stream::Stream()
+Stream::Stream(bool colors)
     : m_flags(0), m_addr_size(4), m_byte_order(endian::InlHostByteOrder()),
-      m_indent_level(0), m_forwarder(*this) {}
+      m_indent_level(0), m_forwarder(*this, colors) {}
 
 // Destructor
 Stream::~Stream() {}
diff --git a/lldb/test/Shell/Driver/TestNoUseColor.test b/lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,3 @@
-# RUN: %lldb --no-use-colors -s %s | FileCheck %s
-settings show use-color
-# CHECK: use-color (boolean) = false
-q
+RUN: not %lldb -b --no-use-colors -o 'settings show use-color' -o 'bogus' 2>&1 | FileCheck %s --check-prefix NOCOLOR
+NOCOLOR: use-color (boolean) = false
+NOCOLOR: error: 'bogus' is not a valid command
diff --git a/lldb/test/Shell/Driver/TestUseColor.test b/lldb/test/Shell/Driver/TestUseColor.test
@@ -0,0 +1,7 @@
+UNSUPPORTED: system-windows
+
+RUN: not %lldb -b -o 'settings set use-color true' -o 'settings show use-color' -o 'bogus' > %t 2>&1
+RUN: cat -e %t | FileCheck %s --check-prefix COLOR
+COLOR: use-color (boolean) = true
+# The [[ confuses FileCheck so regex match it.
+COLOR: {{.+}}0;1;31merror: {{.+}}0m'bogus' is not a valid command
diff --git a/lldb/tools/lldb-test/lldb-test.cpp b/lldb/tools/lldb-test/lldb-test.cpp
@@ -380,7 +380,7 @@ int opts::breakpoint::evaluateBreakpoints(Debugger &Dbg) {
 
     std::string Command = substitute(Line);
     P.formatLine("Command: {0}", Command);
-    CommandReturnObject Result;
+    CommandReturnObject Result(/*colors*/ false);
     if (!Dbg.GetCommandInterpreter().HandleCommand(
             Command.c_str(), /*add_to_history*/ eLazyBoolNo, Result)) {
       P.formatLine("Failed: {0}", Result.GetErrorData());
@@ -530,7 +530,7 @@ Error opts::symbols::findTypes(lldb_private::Module &Module) {
   LanguageSet languages;
   if (!Language.empty())
     languages.Insert(Language::GetLanguageTypeFromString(Language));
-  
+
   DenseSet<SymbolFile *> SearchedFiles;
   TypeMap Map;
   if (!Name.empty())
@@ -1034,7 +1034,7 @@ int opts::irmemorymap::evaluateMemoryMapCommands(Debugger &Dbg) {
 
   // Set up a Process. In order to allocate memory within a target, this
   // process must be alive and must support JIT'ing.
-  CommandReturnObject Result;
+  CommandReturnObject Result(/*colors*/ false);
   Dbg.SetAsyncExecution(false);
   CommandInterpreter &CI = Dbg.GetCommandInterpreter();
   auto IssueCmd = [&](const char *Cmd) -> bool {
@@ -1098,7 +1098,7 @@ int main(int argc, const char *argv[]) {
 
   auto Dbg = lldb_private::Debugger::CreateInstance();
   ModuleList::GetGlobalModuleListProperties().SetEnableExternalLookup(false);
-  CommandReturnObject Result;
+  CommandReturnObject Result(/*colors*/ false);
   Dbg->GetCommandInterpreter().HandleCommand(
       "settings set plugin.process.gdb-remote.packet-timeout 60",
       /*add_to_history*/ eLazyBoolNo, Result);
diff --git a/lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp b/lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -54,7 +54,7 @@ TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
   ASSERT_TRUE(debugger_sp);
 
   ScriptInterpreterLua script_interpreter(*debugger_sp);
-  CommandReturnObject result;
+  CommandReturnObject result(/*colors*/ false);
   EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", &result));
   EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", &result));
   EXPECT_TRUE(result.GetErrorData().startswith(
