[lldb] Add a setting to customize the prompt color

[JDevlieghere]

Users often want to change the look of their prompt and currently the only way to do that is by using ANSI escape codes in the prompt itself. This is not only tedious, it also results in extra whitespace because our Editline wrapper, when computing the cursor column, doesn't ignore the invisible escape codes.

We already have various *-ansi-{prefix,suffix} settings that allow the users to customize the color of auto-suggestions and progress events, using mnemonics like ${ansi.fg.yellow}. This patch brings the same mechanism to the prompt.

rdar://115390406

[llvmbot]

@llvm/pr-subscribers-lldb

Changes

Users often want to change the look of their prompt and currently the only way to do that is by using ANSI escape codes in the prompt itself. This is not only tedious, it also results in extra whitespace because our Editline wrapper, when computing the cursor column, doesn't ignore the invisible escape codes.

We already have various *-ansi-{prefix,suffix} settings that allow the users to customize the color of auto-suggestions and progress events, using mnemonics like ${ansi.fg.yellow}. This patch brings the same mechanism to the prompt.

rdar://115390406

Full diff: https://github.com/llvm/llvm-project/pull/66218.diff

7 Files Affected:

• (modified) lldb/include/lldb/Core/Debugger.h (+4)
• (modified) lldb/include/lldb/Host/Editline.h (+10)
• (modified) lldb/source/Core/CoreProperties.td (+8)
• (modified) lldb/source/Core/Debugger.cpp (+19)
• (modified) lldb/source/Core/IOHandler.cpp (+8-1)
• (modified) lldb/source/Host/common/Editline.cpp (+14-18)
• (modified) lldb/test/API/terminal/TestEditline.py (+12)

diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 5532cace606bfed..6096a42cd89e5a7 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -289,6 +289,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   llvm::StringRef GetPrompt() const;
 
+  llvm::StringRef GetPromptAnsiPrefix() const;
+
+  llvm::StringRef GetPromptAnsiSuffix() const;
+
   void SetPrompt(llvm::StringRef p);
   void SetPrompt(const char *) = delete;
 
diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index 35fd179abb509c3..4a5f2a0d5753a6b 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -211,6 +211,14 @@ class Editline {
     m_fix_indentation_callback_chars = indent_chars;
   }
 
+  void SetPromptAnsiPrefix(std::string prefix) {
+    m_prompt_ansi_prefix = std::move(prefix);
+  }
+
+  void SetPromptAnsiSuffix(std::string suffix) {
+    m_prompt_ansi_suffix = std::move(suffix);
+  }
+
   void SetSuggestionAnsiPrefix(std::string prefix) {
     m_suggestion_ansi_prefix = std::move(prefix);
   }
@@ -399,6 +407,8 @@ class Editline {
   CompleteCallbackType m_completion_callback;
   SuggestionCallbackType m_suggestion_callback;
 
+  std::string m_prompt_ansi_prefix;
+  std::string m_prompt_ansi_suffix;
   std::string m_suggestion_ansi_prefix;
   std::string m_suggestion_ansi_suffix;
 
diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index c7d69676f6e14c0..92884258347e9be 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/lldb/source/Core/CoreProperties.td
@@ -65,6 +65,14 @@ let Definition = "debugger" in {
     DefaultEnumValue<"OptionValueString::eOptionEncodeCharacterEscapeSequences">,
     DefaultStringValue<"(lldb) ">,
     Desc<"The debugger command line prompt displayed for the user.">;
+  def PromptAnsiPrefix: Property<"prompt-ansi-prefix", "String">,
+    Global,
+    DefaultStringValue<"${ansi.faint}">,
+    Desc<"When in a color-enabled terminal, use the ANSI terminal code specified in this format immediately before the prompt.">;
+  def PromptAnsiSuffix: Property<"prompt-ansi-suffix", "String">,
+    Global,
+    DefaultStringValue<"${ansi.normal}">,
+    Desc<"When in a color-enabled terminal, use the ANSI terminal code specified in this format immediately after the prompt.">;
   def ScriptLanguage: Property<"script-lang", "Enum">,
     Global,
     DefaultEnumValue<"eScriptLanguagePython">,
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 7ec1efc64fe9383..de3e17e834b5975 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -235,6 +235,13 @@ Status Debugger::SetPropertyValue(const ExecutionContext *exe_ctx,
       // use-color changed. Ping the prompt so it can reset the ansi terminal
       // codes.
       SetPrompt(GetPrompt());
+    } else if (property_path ==
+                   g_debugger_properties[ePropertyPromptAnsiPrefix].name ||
+               property_path ==
+                   g_debugger_properties[ePropertyPromptAnsiSuffix].name) {
+      // Prompt colors changed. Ping the prompt so it can reset the ansi
+      // terminal codes.
+      SetPrompt(GetPrompt());
     } else if (property_path ==
                g_debugger_properties[ePropertyUseSourceCache].name) {
       // use-source-cache changed. Wipe out the cache contents if it was
@@ -301,6 +308,18 @@ llvm::StringRef Debugger::GetPrompt() const {
       idx, g_debugger_properties[idx].default_cstr_value);
 }
 
+llvm::StringRef Debugger::GetPromptAnsiPrefix() const {
+  const uint32_t idx = ePropertyPromptAnsiPrefix;
+  return GetPropertyAtIndexAs<llvm::StringRef>(
+      idx, g_debugger_properties[idx].default_cstr_value);
+}
+
+llvm::StringRef Debugger::GetPromptAnsiSuffix() const {
+  const uint32_t idx = ePropertyPromptAnsiSuffix;
+  return GetPropertyAtIndexAs<llvm::StringRef>(
+      idx, g_debugger_properties[idx].default_cstr_value);
+}
+
 void Debugger::SetPrompt(llvm::StringRef p) {
   constexpr uint32_t idx = ePropertyPrompt;
   SetPropertyAtIndex(idx, p);
diff --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index ac9a9bb2844594f..7272e5df43fe7e1 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -474,8 +474,15 @@ bool IOHandlerEditline::SetPrompt(llvm::StringRef prompt) {
   m_prompt = std::string(prompt);
 
 #if LLDB_ENABLE_LIBEDIT
-  if (m_editline_up)
+  if (m_editline_up) {
     m_editline_up->SetPrompt(m_prompt.empty() ? nullptr : m_prompt.c_str());
+    if (m_debugger.GetUseColor()) {
+      m_editline_up->SetPromptAnsiPrefix(
+          ansi::FormatAnsiTerminalCodes(m_debugger.GetPromptAnsiPrefix()));
+      m_editline_up->SetPromptAnsiSuffix(
+          ansi::FormatAnsiTerminalCodes(m_debugger.GetPromptAnsiSuffix()));
+    }
+  }
 #endif
   return true;
 }
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 544804db3879eb5..ad16047325c921c 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -52,10 +52,6 @@ int setupterm(char *term, int fildes, int *errret);
 
 /// https://www.ecma-international.org/publications/files/ECMA-ST/Ecma-048.pdf
 #define ESCAPE "\x1b"
-/// Faint, decreased intensity or second colour.
-#define ANSI_FAINT ESCAPE "[2m"
-/// Normal colour or normal intensity (neither bold nor faint).
-#define ANSI_UNFAINT ESCAPE "[0m"
 #define ANSI_CLEAR_BELOW ESCAPE "[J"
 #define ANSI_CLEAR_RIGHT ESCAPE "[K"
 #define ANSI_SET_COLUMN_N ESCAPE "[%dG"
@@ -424,15 +420,13 @@ void Editline::MoveCursor(CursorLocation from, CursorLocation to) {
 void Editline::DisplayInput(int firstIndex) {
   fprintf(m_output_file, ANSI_SET_COLUMN_N ANSI_CLEAR_BELOW, 1);
   int line_count = (int)m_input_lines.size();
-  const char *faint = m_color_prompts ? ANSI_FAINT : "";
-  const char *unfaint = m_color_prompts ? ANSI_UNFAINT : "";
-
   for (int index = firstIndex; index < line_count; index++) {
-    fprintf(m_output_file, "%s"
-                           "%s"
-                           "%s" EditLineStringFormatSpec " ",
-            faint, PromptForIndex(index).c_str(), unfaint,
-            m_input_lines[index].c_str());
+    fprintf(m_output_file,
+            "%s"
+            "%s"
+            "%s" EditLineStringFormatSpec " ",
+            m_prompt_ansi_prefix.c_str(), PromptForIndex(index).c_str(),
+            m_prompt_ansi_suffix.c_str(), m_input_lines[index].c_str());
     if (index < line_count - 1)
       fprintf(m_output_file, "\n");
   }
@@ -541,14 +535,16 @@ unsigned char Editline::RecallHistory(HistoryOperation op) {
 int Editline::GetCharacter(EditLineGetCharType *c) {
   const LineInfoW *info = el_wline(m_editline);
 
-  // Paint a faint version of the desired prompt over the version libedit draws
-  // (will only be requested if colors are supported)
+  // Paint a ANSI formatted version of the desired prompt over the version
+  // libedit draws. (will only be requested if colors are supported)
   if (m_needs_prompt_repaint) {
     MoveCursor(CursorLocation::EditingCursor, CursorLocation::EditingPrompt);
-    fprintf(m_output_file, "%s"
-                           "%s"
-                           "%s",
-            ANSI_FAINT, Prompt(), ANSI_UNFAINT);
+    fprintf(m_output_file,
+            "%s"
+            "%s"
+            "%s",
+            m_prompt_ansi_prefix.c_str(), Prompt(),
+            m_prompt_ansi_suffix.c_str());
     MoveCursor(CursorLocation::EditingPrompt, CursorLocation::EditingCursor);
     m_needs_prompt_repaint = false;
   }
diff --git a/lldb/test/API/terminal/TestEditline.py b/lldb/test/API/terminal/TestEditline.py
index 87fb3cba610c83c..e65e2f74cded6ff 100644
--- a/lldb/test/API/terminal/TestEditline.py
+++ b/lldb/test/API/terminal/TestEditline.py
@@ -44,3 +44,15 @@ def test_left_right_arrow(self):
             )
 
         self.quit()
+
+    @skipIfAsan
+    @skipIfEditlineSupportMissing
+    def test_prompt_color(self):
+        """Test that ANSI escape codes don't break the prompt"""
+        self.launch(use_colors=True)
+        self.child.send('settings set prompt-ansi-prefix "${ansi.fg.red}"\n')
+        # Make sure this change is reflected immediately. Check that the color
+        # is set (31) and the cursor position (8) is correct.
+        # Prompt: (lldb) _
+        # Column: 1....6.8
+        self.child.expect(re.escape("\x1b[31m(lldb) \x1b[0m\x1b[8G"))

[JDevlieghere]

Regarding the column width bug: it's something we'd want to fix anyway. I have a naive implementation here. Probably more important than the ANSI escape characters is UTF-8 which poses a similar issue.

[clayborg]

Any interest in just allowing "settings set prompt" to use the format strings approach that we support in thread-format, frame-format? I see we have a ton of settings that already ansi-prefix and ansi-suffix settings, but it would be easy to just do:

(lldb) setting set prompt "${ansi.fg.red}(lldb) ${ansi.normal}"

It we didn't just expand the above string one time, we could be able to make some really powerful LLDB prompts if we used the format strings:

(lldb) setting set prompt "{pid = ${process.id} }{tid = ${thread.id%tid}} ${ansi.fg.red}(lldb) ${ansi.normal}"

Just an idea, not a requirement for this patch

[JDevlieghere]

Yeah, I considered that in the past (I think I have at least one radar asking for that). It's doable, but will probably require a bit of work:

1. We redraw the prompt a lot. Every time you type a character, we update the cursor column which means redrawing the line and the prompt. We'd have to be smart about doing the substitution only once per "line".
2. You will need access to the debugger, target, process etc. Currently Editline lives in Host, so we'd have to move that out into Core to avoid circular dependencies.
3. Currently we insert an ANSI_FAINT and ANSI_CLEAR before and after the prompt. We'd still need to get rid of that, otherwise users would have to put a CLEAR at the start of the prompt (and possibly have a redundant one at the end).
4. We'd still need to filter out the escape characters in the column width calculation.

All these things can be overcome of course, but for now that's out of scope, at least for me.

[clayborg]

We need to test with colors disabled to ensure the control codes don't show up and then this is good to go

[clayborg]

Can we also test with "use_colors=False" to ensure these sequences don't get inserted?

[kastiglione]

You will need access to the debugger, target, process etc. Currently Editline lives in Host, so we'd have to move that out into Core to avoid circular dependencies.

can we wire up the ansi color formatting without supporting all these other bits of data?

if we're going to eventually support formatting in the prompt, it seems like we should just do that, instead of adding settings and then later removing them.

[JDevlieghere]

can we wire up the ansi color formatting without supporting all these other bits of data?

Yes, but you'd still need to solve (1), (3) and (4) which are the "hard" ones. Doing what you suggests only saves you (2) which is by far the easiest one in the list.