llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: David Spickett (DavidSpickett)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/183558.diff
4 Files Affected:
- (modified) lldb/include/lldb/Utility/AnsiTerminal.h (+15-5)
- (modified) lldb/source/Interpreter/Options.cpp (+2-1)
- (modified) lldb/test/API/commands/help/TestHelp.py (+5-3)
- (modified) lldb/unittests/Utility/AnsiTerminalTest.cpp (+21-6)
``````````diff
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());
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/183558
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
