[Lldb-commits] [lldb] 051a5ae - [lldb/Core] Fix "sticky" long progress messages

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2022-05-18T18:22:46-07:00
New Revision: 051a5ae99824fab879d9aade9d794e60ebc5c2e5

URL: https://github.com/llvm/llvm-project/commit/051a5ae99824fab879d9aade9d794e60ebc5c2e5
DIFF: https://github.com/llvm/llvm-project/commit/051a5ae99824fab879d9aade9d794e60ebc5c2e5.diff

LOG: [lldb/Core] Fix "sticky" long progress messages

When the terminal window is too small, lldb would wrap progress messages
accross multiple lines which would break the progress event handling
code that is supposed to clear the message once the progress is completed.

This causes the progress message to remain on the screen, sometimes partially,
which can be confusing for the user.

To fix this issue, this patch trims the progress message to the terminal
width taking into account the progress counter leading the message for
finite progress events and also the trailing `...`.

rdar://91993836

Differential Revision: https://reviews.llvm.org/D124785

Signed-off-by: Med Ismail Bennani <medismail.bennani at gmail.com>

Added: 
    lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py

Modified: 
    lldb/include/lldb/Core/DebuggerEvents.h
    lldb/source/Core/Debugger.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 8388d5ce66f1c..b1ddb1fc47e2d 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -32,6 +32,7 @@ class ProgressEventData : public EventData {
 
   static const ProgressEventData *GetEventDataFromEvent(const Event *event_ptr);
   uint64_t GetID() const { return m_id; }
+  bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   const std::string &GetMessage() const { return m_message; }

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index ff158dd1cfdfe..5f0b1cc66c0d3 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1852,20 +1852,26 @@ void Debugger::HandleProgressEvent(const lldb::EventSP &event_sp) {
     return;
   }
 
+  // Trim the progress message if it exceeds the window's width and print it.
+  std::string message = data->GetMessage();
+  if (data->IsFinite())
+    message = llvm::formatv("[{0}/{1}] {2}", data->GetCompleted(),
+                            data->GetTotal(), message)
+                  .str();
+
+  // Trim the progress message if it exceeds the window's width and print it.
+  const uint32_t term_width = GetTerminalWidth();
+  const uint32_t ellipsis = 3;
+  if (message.size() + ellipsis >= term_width)
+    message = message.substr(0, term_width - ellipsis);
+
   const bool use_color = GetUseColor();
   llvm::StringRef ansi_prefix = GetShowProgressAnsiPrefix();
   if (!ansi_prefix.empty())
     output->Printf(
         "%s", ansi::FormatAnsiTerminalCodes(ansi_prefix, use_color).c_str());
 
-  // Print the progress message.
-  std::string message = data->GetMessage();
-  if (data->GetTotal() != UINT64_MAX) {
-    output->Printf("[%" PRIu64 "/%" PRIu64 "] %s...", data->GetCompleted(),
-                   data->GetTotal(), message.c_str());
-  } else {
-    output->Printf("%s...", message.c_str());
-  }
+  output->Printf("%s...", message.c_str());
 
   llvm::StringRef ansi_suffix = GetShowProgressAnsiSuffix();
   if (!ansi_suffix.empty())

diff  --git a/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py b/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
new file mode 100644
index 0000000000000..53765b0b37936
--- /dev/null
+++ b/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
@@ -0,0 +1,50 @@
+"""
+Test trimming long progress report in tiny terminal windows
+"""
+
+import os
+import pexpect
+import tempfile
+import re
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestTrimmedProgressReporting(PExpectTest):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def do_test(self, term_width, pattern_list):
+        self.build()
+        # Start with a small window
+        self.launch(use_colors=True)
+        self.expect("set set show-progress true")
+        self.expect("set show show-progress", substrs=["show-progress (boolean) = true"])
+        self.expect("set set term-width " + str(term_width))
+        self.expect("set show term-width", substrs=["term-width (int) = " + str(term_width)])
+
+        self.child.send("file " + self.getBuildArtifact("a.out") + "\n")
+        self.child.expect(pattern_list)
+
+
+    # PExpect uses many timeouts internally and doesn't play well
+    # under ASAN on a loaded machine..
+    @skipIfAsan
+    @skipUnlessDarwin
+    @skipIfEditlineSupportMissing
+    def test_trimmed_progress_message(self):
+        self.do_test(19, ['Locating externa...',
+                          'Loading Apple DW...',
+                          'Parsing symbol t...'])
+
+    # PExpect uses many timeouts internally and doesn't play well
+    # under ASAN on a loaded machine..
+    @skipIfAsan
+    @skipUnlessDarwin
+    @skipIfEditlineSupportMissing
+    def test_long_progress_message(self):
+        self.do_test(80, ['Locating external symbol file for a.out...',
+                          'Loading Apple DWARF index for a.out...',
+                          'Parsing symbol table for dyld...'])