[FormatEntity] Ignore ASCII escape sequences when colors are disabled.

[JDevlieghere]

Summary

This patch makes the FormatEntity honor the debugger's color settings by not inserting ASCII escape sequences when colors are disabled.

[xiaobai]

Seems good to me.

lldb/lit/Settings/TestFrameFormatColor.test

12 ↗

(On Diff #202354)

I don't see any escape code here, I assume phabricator is just not showing them? 😛

[JDevlieghere]

lldb/lit/Settings/TestFrameFormatColor.test

12 ↗

(On Diff #202354)

Yup!

[teemperor]

LGTM, thanks!

[clayborg]

lldb/trunk/include/lldb/Core/FormatEntity.h

44

InsertString is used for more than just escape codes so it is now not named correctly. For a variable formatter string like "(${var.x}, ${var.y})", the "(" is a string, and ", " and ")" are InsertString objects. Any constant string that is part of a formatter. Can you change the name back?

lldb/trunk/include/lldb/Core/FormatEntity.h

 namespace lldb_private {
 class FormatEntity {
 public:
   struct Entry {
     enum class Type {
       Invalid,
       ParentNumber,
       ParentString,
-      InsertString,
+      EscapeCode,
       Root,
       String,
       Scope,
       Variable,
       VariableSynthetic,
       ScriptVariable,
       ScriptVariableSynthetic,
       AddressLoad,

lldb/trunk/lit/Settings/Inputs/main.c

+int foo() { return 0; }
+int main() { return foo(); }

lldb/trunk/lit/Settings/TestFrameFormatColor.test

+# RUN: %clang -g -O0 %S/Inputs/main.c -o %t.out
+# RUN: %lldb -x -b -s %s %t.out | FileCheck %s
+settings set use-color true
+settings set -f frame-format "frame #${frame.index}: \`${ansi.fg.green}{${function.name-with-args}${ansi.normal}\n"
+b foo
+run
+bt
+c
+q
+
+# Check the ASCII escape code
+# CHECK: ␛

lldb/trunk/lit/Settings/TestFrameFormatNoColor.test

+# RUN: %clang -g -O0 %S/Inputs/main.c -o %t.out
+# RUN: %lldb -x -b -s %s %t.out | FileCheck %s
+settings set use-color false
+settings set -f frame-format "frame #${frame.index}: \`${ansi.fg.green}{${function.name-with-args}${ansi.normal}\n"
+b foo
+run
+bt
+c
+q
+
+# Check the ASCII escape code
+# CHECK-NOT: ␛

lldb/trunk/source/Core/FormatEntity.cpp

n, nullptr, FormatEntity::Entry::Type::t, 0,                               \
         static_cast<uint32_t>(llvm::array_lengthof(c)), c, false               \
   }
 #define ENTRY_CHILDREN_KEEP_SEP(n, t, c)                                       \
   {                                                                            \
     n, nullptr, FormatEntity::Entry::Type::t, 0,                               \
         static_cast<uint32_t>(llvm::array_lengthof(c)), c, true                \
   }
 #define ENTRY_STRING(n, s)                                                     \
-  { n, s, FormatEntity::Entry::Type::InsertString, 0, 0, nullptr, false }
+  { n, s, FormatEntity::Entry::Type::EscapeCode, 0, 0, nullptr, false }
 static FormatEntity::Entry::Definition g_string_entry[] = {
     ENTRY("*", ParentString)};
 
 static FormatEntity::Entry::Definition g_addr_entries[] = {
     ENTRY("load", AddressLoad),
     ENTRY("file", AddressFile),
     ENTRY("load", AddressLoadOrFile),
 };
#define ENUM_TO_CSTR(eee)                                                      \
   case FormatEntity::Entry::Type::eee:                                         \
     return #eee
 
 const char *FormatEntity::Entry::TypeToCString(Type t) {
   switch (t) {
     ENUM_TO_CSTR(Invalid);
     ENUM_TO_CSTR(ParentNumber);
     ENUM_TO_CSTR(ParentString);
-    ENUM_TO_CSTR(InsertString);
+    ENUM_TO_CSTR(EscapeCode);
     ENUM_TO_CSTR(Root);
     ENUM_TO_CSTR(String);
     ENUM_TO_CSTR(Scope);
     ENUM_TO_CSTR(Variable);
     ENUM_TO_CSTR(VariableSynthetic);
     ENUM_TO_CSTR(ScriptVariable);
     ENUM_TO_CSTR(ScriptVariableSynthetic);
     ENUM_TO_CSTR(AddressLoad);
bool FormatEntity::Format(const Entry &entry, Stream &s,
                           ValueObject *valobj, bool function_changed,
                           bool initial_function) {
   switch (entry.type) {
   case Entry::Type::Invalid:
   case Entry::Type::ParentNumber: // Only used for
                                   // FormatEntity::Entry::Definition encoding
   case Entry::Type::ParentString: // Only used for
                                   // FormatEntity::Entry::Definition encoding
-  case Entry::Type::InsertString: // Only used for
-                                  // FormatEntity::Entry::Definition encoding
+    return false;
+  case Entry::Type::EscapeCode:
+    if (exe_ctx) {
+      if (Target *target = exe_ctx->GetTargetPtr()) {
+        Debugger &debugger = target->GetDebugger();
+        if (debugger.GetUseColor()) {
+          s.PutCString(entry.string);
+          return true;
+        }
+      }
+    }
     return false;
 
   case Entry::Type::Root:
     for (const auto &child : entry.children) {
       if (!Format(child, s, sc, exe_ctx, addr, valobj, function_changed,
                   initial_function)) {
         return false; // If any item of root fails, then the formatting fails
       }
if (key.equals(entry_def->name) || entry_def->name[0] == '*') {
       case FormatEntity::Entry::Type::ParentString:
         entry.string = format_str.str();
         return error; // Success
 
       case FormatEntity::Entry::Type::ParentNumber:
         entry.number = entry_def->data;
         return error; // Success
 
-      case FormatEntity::Entry::Type::InsertString:
+      case FormatEntity::Entry::Type::EscapeCode:
         entry.type = entry_def->type;
         entry.string = entry_def->string;
         return error; // Success
 
       default:
         entry.type = entry_def->type;
         break;
       }
case '$':
             // or ${svar}
             if (entry.deref) {
               error.SetErrorStringWithFormat(
                   "${%s} can't be dereferenced, only ${var} and ${svar} can.",
                   variable.str().c_str());
               return error;
             }
           }
-          // Check if this entry just wants to insert a constant string value
-          // into the parent_entry, if so, insert the string with AppendText,
-          // else append the entry to the parent_entry.
-          if (entry.type == Entry::Type::InsertString)
-            parent_entry.AppendText(entry.string.c_str());
-          else
-            parent_entry.AppendEntry(std::move(entry));
         }
       }
       break;
     }
   }
   return error;
 }