[LLDB-DAP] SBDebugger don't prefix title on progress updates

lldb/bindings/interface/SBProgressDocstrings.i

@@ -11,7 +11,48 @@ The Progress class helps make sure that progress is correctly reported
 and will always send an initial progress update, updates when
 Progress::Increment() is called, and also will make sure that a progress
 completed update is reported even if the user doesn't explicitly cause one
-to be sent.") lldb::SBProgress;
+to be sent.
+
+Progress can either be deterministic, incrementing up to a known total or non-deterministic
+with an unbounded total. Deterministic is better if you know the items of work in advance, but non-deterministic
+exposes a way to update a user during a long running process that work is taking place.
+
+For all progresses the details provided in the constructor will be sent until an increment detail
+is provided. This detail will also continue to be broadcasted on any subsequent update that doesn't
+specify a new detail. Some implementations differ on throttling updates and this behavior differs primarily
+if the progress is deterministic or non-deterministic. For DAP, non-deterministic update messages have a higher
+throttling rate than deterministic ones.
+
+Below are examples in Python for deterministic and non-deterministic progresses.
+
+    deterministic_progress1 = lldb.SBProgress('Deterministic Progress', 'Detail', 3, lldb.SBDebugger) 
+    for i in range(3): 
+        deterministic_progress1.Increment(1, f'Update {i}') 
+    # The call to Finalize() is a no-op as we already incremented the right amount of 
+    # times and caused the end event to be sent. 
+    deterministic_progress1.Finalize()
+
+    deterministic_progress2 = lldb.SBProgress('Deterministic Progress', 'Detail', 10, lldb.SBDebugger) 
+    for i in range(3): 
+        deterministic_progress2.Increment(1, f'Update {i}')      
+    # Cause the progress end event to be sent even if we didn't increment the right 
+    # number of times. Real world examples would be in a try-finally block to ensure
+    # progress clean-up.
+    deterministic_progress2.Finalize() 
+
+If you don't call Finalize() when the progress is not done, the progress object will eventually get
+garbage collected by the Python runtime, the end event will eventually get sent, but it is best not to 
+rely on the garbage collection when using lldb.SBProgress.
+
+Non-deterministic progresses behave the same, but omit the total in the constructor.
+
+    non_deterministic_progress = lldb.SBProgress('Non deterministic progress, 'Detail', lldb.SBDebugger)
+    for i in range(10):
+        non_deterministic_progress.Increment(1)
+    # Explicitly send a progressEnd, otherwise this will be sent
+    # when the python runtime cleans up this object.
+    non_deterministic_progress.Finalize()
+") lldb::SBProgress;    
 
 %feature("docstring",
 "Finalize the SBProgress, which will cause a progress end event to be emitted. This 

lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py

@@ -37,7 +37,11 @@ def create_options(cls):
         )
 
         parser.add_option(
-            "--total", dest="total", help="Total to count up.", type="int"
+            "--total",
+            dest="total",
+            help="Total items in this progress object. When this option is not specified, this will be an indeterminate progress.",
+            type="int",
+            default=None,
         )
 
         parser.add_option(
@@ -47,6 +51,14 @@ def create_options(cls):
             type="int",
         )
 
+        parser.add_option(
+            "--no-details",
+            dest="no_details",
+            help="Do not display details",
+            action="store_true",
+            default=False,
+        )
+
         return parser
 
     def get_short_help(self):
@@ -68,12 +80,30 @@ def __call__(self, debugger, command, exe_ctx, result):
             return
 
         total = cmd_options.total
-        progress = lldb.SBProgress("Progress tester", "Detail", total, debugger)
+        if total is None:
+            progress = lldb.SBProgress(
+                "Progress tester", "Initial Indeterminate Detail", debugger
+            )
+        else:
+            progress = lldb.SBProgress(
+                "Progress tester", "Initial Detail", total, debugger
+            )
+
+        # Check to see if total is set to None to indicate an indeterminate progress
+        # then default to 10 steps.
+        if total is None:
+            total = 10
 
         for i in range(1, total):
-            progress.Increment(1, f"Step {i}")
+            if cmd_options.no_details:
+                progress.Increment(1)
+            else:
+                progress.Increment(1, f"Step {i}")
             time.sleep(cmd_options.seconds)
 
+        # Not required for deterministic progress, but required for indeterminate progress.
+        progress.Finalize()
+
 
 def __lldb_init_module(debugger, dict):
     # Register all classes that have a register_lldb_command method

lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py

@@ -4,21 +4,54 @@
 
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
+import json
 import os
 import time
 
 import lldbdap_testcase
 
 
 class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase):
+    def verify_progress_events(
+        self,
+        expected_title,
+        expected_message=None,
+        expected_not_in_message=None,
+        only_verify_first_update=False,
+    ):
+        self.dap_server.wait_for_event("progressEnd", 15)
+        self.assertTrue(len(self.dap_server.progress_events) > 0)
+        start_found = False
+        update_found = False
+        end_found = False
+        for event in self.dap_server.progress_events:
+            event_type = event["event"]
+            if "progressStart" in event_type:
+                title = event["body"]["title"]
+                self.assertIn(expected_title, title)
+                start_found = True
+            if "progressUpdate" in event_type:
+                message = event["body"]["message"]
+                if only_verify_first_update and update_found:
+                    continue
+                if expected_message is not None:
+                    self.assertIn(expected_message, message)
+                if expected_not_in_message is not None:
+                    self.assertNotIn(expected_not_in_message, message)
+                update_found = True
+            if "progressEnd" in event_type:
+                end_found = True
+
+        self.assertTrue(start_found)
+        self.assertTrue(update_found)
+        self.assertTrue(end_found)
+
     @skipIfWindows
     def test_output(self):
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
         progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
-        print(f"Progress emitter path: {progress_emitter}")
         source = "main.cpp"
-        # Set breakpoint in the thread function so we can step the threads
         breakpoint_ids = self.set_source_breakpoints(
             source, [line_number(source, "// break here")]
         )
@@ -30,20 +63,73 @@ def test_output(self):
             "`test-progress --total 3 --seconds 1", context="repl"
         )
 
-        self.dap_server.wait_for_event("progressEnd", 15)
-        # Expect at least a start, an update, and end event
-        # However because the underlying Progress instance is an RAII object and we can't guaruntee
-        # it's deterministic destruction in the python API, we verify just start and update
-        # otherwise this test could be flakey.
-        self.assertTrue(len(self.dap_server.progress_events) > 0)
-        start_found = False
-        update_found = False
-        for event in self.dap_server.progress_events:
-            event_type = event["event"]
-            if "progressStart" in event_type:
-                start_found = True
-            if "progressUpdate" in event_type:
-                update_found = True
+        self.verify_progress_events(
+            expected_title="Progress tester",
+            expected_not_in_message="Progress tester",
+        )
 
-        self.assertTrue(start_found)
-        self.assertTrue(update_found)
+    @skipIfWindows
+    def test_output_nodetails(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
+        source = "main.cpp"
+        breakpoint_ids = self.set_source_breakpoints(
+            source, [line_number(source, "// break here")]
+        )
+        self.continue_to_breakpoints(breakpoint_ids)
+        self.dap_server.request_evaluate(
+            f"`command script import {progress_emitter}", context="repl"
+        )
+        self.dap_server.request_evaluate(
+            "`test-progress --total 3 --seconds 1 --no-details", context="repl"
+        )
+
+        self.verify_progress_events(
+            expected_title="Progress tester",
+            expected_message="Initial Detail",
+        )
+
+    @skipIfWindows
+    def test_output_indeterminate(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
+        source = "main.cpp"
+        breakpoint_ids = self.set_source_breakpoints(
+            source, [line_number(source, "// break here")]
+        )
+        self.continue_to_breakpoints(breakpoint_ids)
+        self.dap_server.request_evaluate(
+            f"`command script import {progress_emitter}", context="repl"
+        )
+        self.dap_server.request_evaluate("`test-progress --seconds 1", context="repl")
+
+        self.verify_progress_events(
+            expected_title="Progress tester: Initial Indeterminate Detail",
+            expected_message="Step 1",
+            only_verify_first_update=True,
+        )
+
+    @skipIfWindows
+    def test_output_nodetails_indeterminate(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
+        source = "main.cpp"
+        breakpoint_ids = self.set_source_breakpoints(
+            source, [line_number(source, "// break here")]
+        )
+        self.dap_server.request_evaluate(
+            f"`command script import {progress_emitter}", context="repl"
+        )
+
+        self.dap_server.request_evaluate(
+            "`test-progress --seconds 1 --no-details", context="repl"
+        )
+
+        self.verify_progress_events(
+            expected_title="Progress tester: Initial Indeterminate Detail",
+            expected_message="Initial Indeterminate Detail",
+            only_verify_first_update=True,
+        )

lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp

@@ -18,8 +18,32 @@ using namespace lldb;
 
 namespace lldb_dap {
 
-static void ProgressEventThreadFunction(DAP &dap) {
-  llvm::set_thread_name(dap.name + ".progress_handler");
+static std::string GetStringFromStructuredData(lldb::SBStructuredData &data,
+                                               const char *key) {
+  lldb::SBStructuredData keyValue = data.GetValueForKey(key);
+  if (!keyValue)
+    return std::string();
+
+  const size_t length = keyValue.GetStringValue(nullptr, 0);
+
+  if (length == 0)
+    return std::string();
+
+  std::string str(length + 1, 0);
+  keyValue.GetStringValue(&str[0], length + 1);
+  return str;
+}
+
+static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
+                                          const char *key) {
+  lldb::SBStructuredData keyValue = data.GetValueForKey(key);
+
+  if (!keyValue.IsValid())
+    return 0;
+  return keyValue.GetUnsignedIntegerValue();
+}
+
+void ProgressEventThreadFunction(DAP &dap) {
   lldb::SBListener listener("lldb-dap.progress.listener");
   dap.debugger.GetBroadcaster().AddListener(
       listener, lldb::SBDebugger::eBroadcastBitProgress |
@@ -35,14 +59,47 @@ static void ProgressEventThreadFunction(DAP &dap) {
           done = true;
         }
       } else {
-        uint64_t progress_id = 0;
-        uint64_t completed = 0;
-        uint64_t total = 0;
-        bool is_debugger_specific = false;
-        const char *message = lldb::SBDebugger::GetProgressFromEvent(
-            event, progress_id, completed, total, is_debugger_specific);
-        if (message)
-          dap.SendProgressEvent(progress_id, message, completed, total);
+        lldb::SBStructuredData data =
+            lldb::SBDebugger::GetProgressDataFromEvent(event);
+
+        const uint64_t progress_id =
+            GetUintFromStructuredData(data, "progress_id");
+        const uint64_t completed = GetUintFromStructuredData(data, "completed");
+        const uint64_t total = GetUintFromStructuredData(data, "total");
+        const std::string details =
+            GetStringFromStructuredData(data, "details");
+
+        if (completed == 0) {
+          if (total == UINT64_MAX) {
+            // This progress is non deterministic and won't get updated until it
+            // is completed. Send the "message" which will be the combined title
+            // and detail. The only other progress event for thus
+            // non-deterministic progress will be the completed event So there
+            // will be no need to update the detail.
+            const std::string message =
+                GetStringFromStructuredData(data, "message");
+            dap.SendProgressEvent(progress_id, message.c_str(), completed,
+                                  total);
+          } else {
+            // This progress is deterministic and will receive updates,
+            // on the progress creation event VSCode will save the message in
+            // the create packet and use that as the title, so we send just the
+            // title in the progressCreate packet followed immediately by a
+            // detail packet, if there is any detail.
+            const std::string title =
+                GetStringFromStructuredData(data, "title");
+            dap.SendProgressEvent(progress_id, title.c_str(), completed, total);
+            if (!details.empty())
+              dap.SendProgressEvent(progress_id, details.c_str(), completed,
+                                    total);
+          }
+        } else {
+          // This progress event is either the end of the progress dialog, or an
+          // update with possible detail. The "detail" string we send to VS Code
+          // will be appended to the progress dialog's initial text from when it
+          // was created.
+          dap.SendProgressEvent(progress_id, details.c_str(), completed, total);
+        }
       }
     }
   }