https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/183558
>From d32e7b182843b3e9a4130c95270f74c7784bfe1c Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 26 Feb 2026 15:48:42 +0000 Subject: [PATCH 1/3] [lldb] Indent option help with ANSI cursor codes when possible. This avoids formatting empty space when a range of text formatted by ANSI codes is split across lines. This is not currently done in any option, but the `${...}` syntax we have does support marking any range of text, so it could be done in future, and fixing it is simple. As an example, if I change a breakpoint option: ``` "${S}et the breakpoint only in this shared library. Can repeat " - "this option multiple times to specify multiple shared libraries.">; + "this option multiple ${times to specify multiple} shared libraries.">; ``` This applies the underline to words that will be split across lines. In the outputs below, `^` represents an underlined character. With spaces: ``` -s <shlib-name> ( --shlib <shlib-name> ) Set the breakpoint only in this shared library. Can repeat this option multiple times to ^^^^^^^^ specify multiple shared libraries. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` The indent and the text are underlined, this is not what we want. With cursor movement: ``` -s <shlib-name> ( --shlib <shlib-name> ) Set the breakpoint only in this shared library. Can repeat this option multiple times to ^^^^^^^^ specify multiple shared libraries. ^^^^^^^^^^^^^^^^ ``` Only the text is underlined, which is correct. If we are not allowed to use ANSI (use-color is off), then the descriptions will be stripped of ANSI anyway, so this is not a problem. --- lldb/include/lldb/Utility/AnsiTerminal.h | 20 +++++++++++---- lldb/source/Interpreter/Options.cpp | 3 ++- lldb/test/API/commands/help/TestHelp.py | 8 +++--- lldb/unittests/Utility/AnsiTerminalTest.cpp | 27 ++++++++++++++++----- 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/lldb/include/lldb/Utility/AnsiTerminal.h b/lldb/include/lldb/Utility/AnsiTerminal.h index 4ab6ef1eb1be7..8dddb2a487b13 100644 --- a/lldb/include/lldb/Utility/AnsiTerminal.h +++ b/lldb/include/lldb/Utility/AnsiTerminal.h @@ -74,6 +74,8 @@ // Cursor Position, set cursor to position [l, c] (default = [1, 1]). #define ANSI_CSI_CUP(...) ANSI_ESC_START #__VA_ARGS__ "H" +// Cursor Position, move cursor forward N columns. +#define ANSI_CSI_CUF(N) (ANSI_ESC_START + N + "C") // Reset cursor to position. #define ANSI_CSI_RESET_CURSOR ANSI_CSI_CUP() // Erase In Display. @@ -405,11 +407,9 @@ inline std::string TrimAndPad(llvm::StringRef str, size_t visible_length, // Output text that may contain ANSI codes, word wrapped (wrapped at whitespace) // to the given stream. The indent level of the stream is counted towards the // output line length. -// FIXME: If an ANSI code is applied to multiple words and those words are split -// across lines, the code will apply to the indentation as well as the -// text. inline void OutputWordWrappedLines(Stream &strm, llvm::StringRef text, - uint32_t output_max_columns) { + uint32_t output_max_columns, + bool use_color) { // We will indent using the stream, so leading whitespace is not significant. text = text.ltrim(); if (text.empty()) @@ -425,7 +425,17 @@ inline void OutputWordWrappedLines(Stream &strm, llvm::StringRef text, if (!first_line) strm.EOL(); first_line = false; - strm.Indent(split); + + if (use_color) { + // If we are allowed to use colour (aka ANSI codes), we can indent using + // ANSI cursor movement. This means that if an ANSI formatted range of + // text is split across two lines, the indentation is not also formatted. + // Which it would be if we just emitted spaces. + const std::string ansi_indent = + ANSI_CSI_CUF(std::to_string(strm.GetIndentLevel())); + strm << ansi_indent << split; + } else + strm.Indent(split); text = text.drop_front(split.size()).ltrim(); } diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp index 0bda2a912e1a1..e87426c48165e 100644 --- a/lldb/source/Interpreter/Options.cpp +++ b/lldb/source/Interpreter/Options.cpp @@ -275,7 +275,8 @@ void Options::OutputFormattedUsageText(Stream &strm, actual_text.append( ansi::FormatAnsiTerminalCodes(option_def.usage_text, use_color)); - ansi::OutputWordWrappedLines(strm, actual_text, output_max_columns); + ansi::OutputWordWrappedLines(strm, actual_text, output_max_columns, + use_color); } bool Options::SupportsLongOption(const char *long_option) { diff --git a/lldb/test/API/commands/help/TestHelp.py b/lldb/test/API/commands/help/TestHelp.py index cb6e9473c1047..1ef6bc0e1cd43 100644 --- a/lldb/test/API/commands/help/TestHelp.py +++ b/lldb/test/API/commands/help/TestHelp.py @@ -325,6 +325,8 @@ def test_help_option_description_terminal_width_no_ansi(self): def test_help_option_description_terminal_width_with_ansi(self): """Test that help on commands formats option descriptions that include ANSI codes acccording to the terminal width.""" + # Note that because color is enabled, we will use ANSI cursor codes to + # indent, rather than spaces. self.runCmd("settings set use-color on") # Should fit on one line. @@ -334,7 +336,7 @@ def test_help_option_description_terminal_width_with_ansi(self): matching=True, patterns=[ # The "S" of "Set" is underlined. - r"\s+\x1b\[4mS\x1b\[0met the breakpoint only in this shared library. Can repeat this option multiple times to specify multiple shared libraries.\n" + r"\x1b\[12C\x1b\[4mS\x1b\[0met the breakpoint only in this shared library. Can repeat this option multiple times to specify multiple shared libraries.\n" ], ) @@ -343,8 +345,8 @@ def test_help_option_description_terminal_width_with_ansi(self): "help breakpoint set", matching=True, patterns=[ - r"\s+\x1b\[4mS\x1b\[0met the breakpoint only in this shared library. Can repeat this option multiple times\n" - r"\s+to specify multiple shared libraries.\n" + r"\x1b\[12C\x1b\[4mS\x1b\[0met the breakpoint only in this shared library. Can repeat this option multiple times\n" + r"\x1b\[12Cto specify multiple shared libraries.\n" ], ) diff --git a/lldb/unittests/Utility/AnsiTerminalTest.cpp b/lldb/unittests/Utility/AnsiTerminalTest.cpp index 6027b21482bdc..d04874a848422 100644 --- a/lldb/unittests/Utility/AnsiTerminalTest.cpp +++ b/lldb/unittests/Utility/AnsiTerminalTest.cpp @@ -260,7 +260,8 @@ static void TestLines(const std::string &input, int indent, const llvm::StringRef &expected) { StreamString strm; strm.SetIndentLevel(indent); - ansi::OutputWordWrappedLines(strm, input, output_max_columns); + ansi::OutputWordWrappedLines(strm, input, output_max_columns, + /*use_color=*/false); EXPECT_EQ(expected, strm.GetString()); } @@ -300,9 +301,23 @@ TEST(AnsiTerminal, OutputWordWrappedLines) { // Must remove the spaces from the end of the first line. TestLines("The quick brown fox.", 0, 15, "The quick\nbrown fox.\n"); - // FIXME: ANSI codes applied to > 1 word end up applying to all those words - // and the indent if those words are split up. We should use cursor - // positioning to do the indentation instead. - TestLines("\x1B[4mabc def\x1B[0m ghi", 2, 6, - " \x1B[4mabc\n def\x1B[0m\n ghi\n"); + // If ANSI formatting is applied to multiple words, that range of words may + // be split over multiple lines. + StreamString indented_strm; + indented_strm.SetIndentLevel(2); + ansi::OutputWordWrappedLines(indented_strm, "\x1B[4mabc def\x1B[0m ghi", 6, + /*use_color=*/false); + // The two spaces before "def" would have the previous ANSI code applied to + // them. + EXPECT_EQ(" \x1B[4mabc\n def\x1B[0m\n ghi\n", indented_strm.GetString()); + + // If we can emit ANSI, we can use cursor positions to skip forward, + // which leaves the indent unformatted. + // (in normal use the inputs are command descriptions, which already have + // ANSI removed if the terminal does not support it) + indented_strm.Clear(); + ansi::OutputWordWrappedLines(indented_strm, "\x1B[4mabc def\x1B[0m ghi", 6, + /*use_color=*/true); + EXPECT_EQ("\x1B[2C\x1B[4mabc\n\x1B[2Cdef\x1B[0m\n\x1B[2Cghi\n", + indented_strm.GetString()); } >From c2a344d840558df69c5bb3fec3da985ccc0cb9af Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 26 Feb 2026 16:10:58 +0000 Subject: [PATCH 2/3] misc cleanup --- lldb/include/lldb/Utility/AnsiTerminal.h | 4 ++-- lldb/test/API/commands/help/TestHelp.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lldb/include/lldb/Utility/AnsiTerminal.h b/lldb/include/lldb/Utility/AnsiTerminal.h index 8dddb2a487b13..e2768dabcc31f 100644 --- a/lldb/include/lldb/Utility/AnsiTerminal.h +++ b/lldb/include/lldb/Utility/AnsiTerminal.h @@ -419,6 +419,8 @@ inline void OutputWordWrappedLines(Stream &strm, llvm::StringRef text, const uint32_t max_text_width = output_max_columns - strm.GetIndentLevel() - 1; bool first_line = true; + const std::string ansi_indent = + ANSI_CSI_CUF(std::to_string(strm.GetIndentLevel())); while (!text.empty()) { std::string split = TrimAtWordBoundary(text, max_text_width); @@ -431,8 +433,6 @@ inline void OutputWordWrappedLines(Stream &strm, llvm::StringRef text, // ANSI cursor movement. This means that if an ANSI formatted range of // text is split across two lines, the indentation is not also formatted. // Which it would be if we just emitted spaces. - const std::string ansi_indent = - ANSI_CSI_CUF(std::to_string(strm.GetIndentLevel())); strm << ansi_indent << split; } else strm.Indent(split); diff --git a/lldb/test/API/commands/help/TestHelp.py b/lldb/test/API/commands/help/TestHelp.py index 1ef6bc0e1cd43..8423d410ca306 100644 --- a/lldb/test/API/commands/help/TestHelp.py +++ b/lldb/test/API/commands/help/TestHelp.py @@ -334,9 +334,9 @@ def test_help_option_description_terminal_width_with_ansi(self): self.expect( "help breakpoint set", matching=True, - patterns=[ + substrs=[ # The "S" of "Set" is underlined. - r"\x1b\[12C\x1b\[4mS\x1b\[0met the breakpoint only in this shared library. Can repeat this option multiple times to specify multiple shared libraries.\n" + "\x1b[12C\x1b[4mS\x1b[0met the breakpoint only in this shared library. Can repeat this option multiple times to specify multiple shared libraries.\n" ], ) @@ -344,9 +344,9 @@ def test_help_option_description_terminal_width_with_ansi(self): self.expect( "help breakpoint set", matching=True, - patterns=[ - r"\x1b\[12C\x1b\[4mS\x1b\[0met the breakpoint only in this shared library. Can repeat this option multiple times\n" - r"\x1b\[12Cto specify multiple shared libraries.\n" + substrs=[ + "\x1b[12C\x1b[4mS\x1b[0met the breakpoint only in this shared library. Can repeat this option multiple times\n" + "\x1b[12Cto specify multiple shared libraries.\n" ], ) >From 7e5b53b0b0726c712d490bc4e94afd2790a066ff Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Mon, 2 Mar 2026 09:51:27 +0000 Subject: [PATCH 3/3] Apply suggestion from @JDevlieghere Co-authored-by: Jonas Devlieghere <[email protected]> --- lldb/include/lldb/Utility/AnsiTerminal.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/include/lldb/Utility/AnsiTerminal.h b/lldb/include/lldb/Utility/AnsiTerminal.h index e2768dabcc31f..68054454dc202 100644 --- a/lldb/include/lldb/Utility/AnsiTerminal.h +++ b/lldb/include/lldb/Utility/AnsiTerminal.h @@ -434,8 +434,9 @@ inline void OutputWordWrappedLines(Stream &strm, llvm::StringRef text, // text is split across two lines, the indentation is not also formatted. // Which it would be if we just emitted spaces. strm << ansi_indent << split; - } else + } else { strm.Indent(split); + } text = text.drop_front(split.size()).ltrim(); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
