https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/124648
>From 12ff645735c1dbf51e58b9f80d4e3e13a0babdf5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Mon, 27 Jan 2025 13:41:58 -0800
Subject: [PATCH 1/9] Only include title on the first message
---
lldb/include/lldb/Core/DebuggerEvents.h | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h
b/lldb/include/lldb/Core/DebuggerEvents.h
index 49a4ecf8e537e..52e4f77d7637d 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -44,12 +44,15 @@ class ProgressEventData : public EventData {
uint64_t GetCompleted() const { return m_completed; }
uint64_t GetTotal() const { return m_total; }
std::string GetMessage() const {
- std::string message = m_title;
- if (!m_details.empty()) {
- message.append(": ");
- message.append(m_details);
- }
- return message;
+ if (m_completed == 0) {
+ std::string message = m_title;
+ if (!m_details.empty()) {
+ message.append(": ");
+ message.append(m_details);
+ }
+ return message;
+ } else
+ return !m_details.empty() ? m_details : std::string();
}
const std::string &GetTitle() const { return m_title; }
const std::string &GetDetails() const { return m_details; }
>From 82ed76ae3b6bef176bf54dd1031f0cb9c95081c1 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Mon, 27 Jan 2025 14:48:01 -0800
Subject: [PATCH 2/9] Add comment explaining if and add a test
---
lldb/include/lldb/Core/DebuggerEvents.h | 1 +
lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h
b/lldb/include/lldb/Core/DebuggerEvents.h
index 52e4f77d7637d..ca06ee835fde3 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -44,6 +44,7 @@ class ProgressEventData : public EventData {
uint64_t GetCompleted() const { return m_completed; }
uint64_t GetTotal() const { return m_total; }
std::string GetMessage() const {
+ // Only put the title in the message of the progress create event.
if (m_completed == 0) {
std::string message = m_title;
if (!m_details.empty()) {
diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
index 36c0cef9c4714..945c3f7633364 100755
--- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -41,8 +41,13 @@ def test_output(self):
for event in self.dap_server.progress_events:
event_type = event["event"]
if "progressStart" in event_type:
+ title = event["body"]["title"]
+ self.assertIn("Progress tester", title)
start_found = True
if "progressUpdate" in event_type:
+ message = event["body"]["message"]
+ print(f"Progress update: {message}")
+ self.assertNotIn("Progres tester", message)
update_found = True
self.assertTrue(start_found)
>From e15090782a93e07e8a260a0594ca02430e68e09f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 30 Jan 2025 10:04:04 -0800
Subject: [PATCH 3/9] Migrate to structured data
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D68927453
---
lldb/include/lldb/Core/DebuggerEvents.h | 16 +++-----
lldb/tools/lldb-dap/lldb-dap.cpp | 54 +++++++++++++++++++++----
2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h
b/lldb/include/lldb/Core/DebuggerEvents.h
index ca06ee835fde3..49a4ecf8e537e 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -44,16 +44,12 @@ class ProgressEventData : public EventData {
uint64_t GetCompleted() const { return m_completed; }
uint64_t GetTotal() const { return m_total; }
std::string GetMessage() const {
- // Only put the title in the message of the progress create event.
- if (m_completed == 0) {
- std::string message = m_title;
- if (!m_details.empty()) {
- message.append(": ");
- message.append(m_details);
- }
- return message;
- } else
- return !m_details.empty() ? m_details : std::string();
+ std::string message = m_title;
+ if (!m_details.empty()) {
+ message.append(": ");
+ message.append(m_details);
+ }
+ return message;
}
const std::string &GetTitle() const { return m_title; }
const std::string &GetDetails() const { return m_details; }
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e323990d8b6ed..4b3460190a7c9 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -55,6 +55,7 @@
#include <thread>
#include <vector>
+#include <iostream>
#if defined(_WIN32)
// We need to #define NOMINMAX in order to skip `min()` and `max()` macro
// definitions that conflict with other system headers.
@@ -412,6 +413,30 @@ void SendStdOutStdErr(DAP &dap, lldb::SBProcess &process) {
dap.SendOutput(OutputType::Stderr, llvm::StringRef(buffer, count));
}
+static std::string GetStringFromStructuredData(lldb::SBStructuredData &data,
+ const char *key) {
+ lldb::SBStructuredData keyValue = data.GetValueForKey(key);
+ if (!keyValue)
+ return std::string();
+
+ size_t size = keyValue.GetStringValue(nullptr, 0);
+ std::cout << "Size for " << key << " " << size << std::endl;
+ std::string stringValue;
+ stringValue.resize(size);
+ keyValue.GetStringValue(&stringValue[0], size + 1);
+ std::cout << "String value after: " << stringValue << std::endl;
+ return stringValue;
+}
+
+static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
+ const char *key) {
+ lldb::SBStructuredData keyValue = data.GetValueForKey(key);
+
+ if (!keyValue.IsValid())
+ return -1;
+ return keyValue.GetUnsignedIntegerValue();
+}
+
void ProgressEventThreadFunction(DAP &dap) {
lldb::SBListener listener("lldb-dap.progress.listener");
dap.debugger.GetBroadcaster().AddListener(
@@ -428,14 +453,27 @@ 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);
+
+ uint64_t progress_id = GetUintFromStructuredData(data, "progress_id");
+ uint64_t completed = GetUintFromStructuredData(data, "completed");
+ uint64_t total = GetUintFromStructuredData(data, "total");
+ std::string message;
+ // Include the title on the first event.
+ if (completed == 0) {
+ std::string title = GetStringFromStructuredData(data, "title");
+ message += title;
+ message += ": ";
+ }
+
+ std::string details = GetStringFromStructuredData(data, "details");
+ message += details;
+ // Verbose check, but we get -1 for the uint64 on failure to read
+ // so we check everything before broadcasting an event.
+ if (message.length() > 0 && progress_id > 0 && total >= 0 &&
+ completed >= 0)
+ dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
}
}
}
>From d616814939c8af59fae20e8994d3af4f37b27232 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 18 Feb 2025 11:28:58 -0800
Subject: [PATCH 4/9] Feedback from Jonas, and cleanup dev logging
---
lldb/tools/lldb-dap/lldb-dap.cpp | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 4b3460190a7c9..b08ab08d9a8b3 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -55,7 +55,6 @@
#include <thread>
#include <vector>
-#include <iostream>
#if defined(_WIN32)
// We need to #define NOMINMAX in order to skip `min()` and `max()` macro
// definitions that conflict with other system headers.
@@ -419,13 +418,10 @@ static std::string
GetStringFromStructuredData(lldb::SBStructuredData &data,
if (!keyValue)
return std::string();
- size_t size = keyValue.GetStringValue(nullptr, 0);
- std::cout << "Size for " << key << " " << size << std::endl;
- std::string stringValue;
- stringValue.resize(size);
- keyValue.GetStringValue(&stringValue[0], size + 1);
- std::cout << "String value after: " << stringValue << std::endl;
- return stringValue;
+ const size_t length = keyValue.GetStringValue(nullptr, 0);
+ std::string str(length + 1, 0);
+ keyValue.GetStringValue(&str[0], length);
+ return str;
}
static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
@@ -433,7 +429,7 @@ static uint64_t
GetUintFromStructuredData(lldb::SBStructuredData &data,
lldb::SBStructuredData keyValue = data.GetValueForKey(key);
if (!keyValue.IsValid())
- return -1;
+ return 0;
return keyValue.GetUnsignedIntegerValue();
}
@@ -456,23 +452,24 @@ void ProgressEventThreadFunction(DAP &dap) {
lldb::SBStructuredData data =
lldb::SBDebugger::GetProgressDataFromEvent(event);
- uint64_t progress_id = GetUintFromStructuredData(data, "progress_id");
- uint64_t completed = GetUintFromStructuredData(data, "completed");
- uint64_t total = GetUintFromStructuredData(data, "total");
+ 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");
std::string message;
// Include the title on the first event.
if (completed == 0) {
- std::string title = GetStringFromStructuredData(data, "title");
+ const std::string title = GetStringFromStructuredData(data, "title");
message += title;
message += ": ";
}
- std::string details = GetStringFromStructuredData(data, "details");
message += details;
// Verbose check, but we get -1 for the uint64 on failure to read
// so we check everything before broadcasting an event.
- if (message.length() > 0 && progress_id > 0 && total >= 0 &&
- completed >= 0)
+ if (message.length() > 0)
dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
}
}
>From f79d7bdd4a3d4062cc8c144c5bb1b1f824087fc9 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Tue, 18 Feb 2025 14:04:53 -0800
Subject: [PATCH 5/9] Fix test by making sure we include up to the last
character
---
lldb/tools/lldb-dap/lldb-dap.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index b08ab08d9a8b3..3805eb3f55879 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -419,8 +419,12 @@ static std::string
GetStringFromStructuredData(lldb::SBStructuredData &data,
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);
+ keyValue.GetStringValue(&str[0], length + 1);
return str;
}
>From d96b3089088871ebccf7306ac2550222a7efafaa Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 19 Feb 2025 12:49:20 -0800
Subject: [PATCH 6/9] Fix bug in SBProgress, add new test cases, and refactor
to support TITLE: DETAIL in all scenarios
---
lldb/source/API/SBProgress.cpp | 5 +-
.../lldb-dap/progress/Progress_emitter.py | 29 +++-
.../lldb-dap/progress/TestDAP_Progress.py | 127 +++++++++++++++++-
lldb/tools/lldb-dap/lldb-dap.cpp | 29 ++--
4 files changed, 174 insertions(+), 16 deletions(-)
diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp
index e67e289a60eff..403900aed4a8c 100644
--- a/lldb/source/API/SBProgress.cpp
+++ b/lldb/source/API/SBProgress.cpp
@@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default;
void SBProgress::Increment(uint64_t amount, const char *description) {
LLDB_INSTRUMENT_VA(amount, description);
- m_opaque_up->Increment(amount, description);
+ std::optional<std::string> description_opt;
+ if (description)
+ description_opt = description;
+ m_opaque_up->Increment(amount, description_opt);
}
lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; }
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
index 7f4055cab9ddd..e962b1a10d402 100644
--- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -37,7 +37,10 @@ def create_options(cls):
)
parser.add_option(
- "--total", dest="total", help="Total to count up.", type="int"
+ "--total",
+ dest="total",
+ help="Total to count up, use -1 to identify as indeterminate",
+ type="int",
)
parser.add_option(
@@ -47,6 +50,13 @@ def create_options(cls):
type="int",
)
+ parser.add_option(
+ "--no-details",
+ dest="no_details",
+ help="Do not display details",
+ action="store_true",
+ )
+
return parser
def get_short_help(self):
@@ -68,10 +78,23 @@ def __call__(self, debugger, command, exe_ctx, result):
return
total = cmd_options.total
- progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
+ if total == -1:
+ progress = lldb.SBProgress(
+ "Progress tester", "Indeterminate Detail", debugger
+ )
+ else:
+ progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
+
+ # Check to see if total is set to -1 to indicate an indeterminate
progress
+ # then default to 10 steps.
+ if total == -1:
+ 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)
diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
index 945c3f7633364..2611df042f677 100755
--- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -4,6 +4,7 @@
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
+import json
import os
import time
@@ -18,7 +19,6 @@ def test_output(self):
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")]
)
@@ -52,3 +52,128 @@ def test_output(self):
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")
+ print(f"Progress emitter path: {progress_emitter}")
+ 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.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:
+ title = event["body"]["title"]
+ self.assertIn("Progress tester", title)
+ start_found = True
+ if "progressUpdate" in event_type:
+ message = event["body"]["message"]
+ # We send an update on first message to set the details, so
ignore the first update
+ if not update_found:
+ self.assertTrue(message == "", message)
+ update_found = True
+
+ self.assertTrue(start_found)
+ self.assertTrue(update_found)
+
+ @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")
+ print(f"Progress emitter path: {progress_emitter}")
+ 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 -1 --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:
+ title = event["body"]["title"]
+ self.assertIn("Progress tester", title)
+ start_found = True
+ if "progressUpdate" in event_type:
+ message = event["body"]["message"]
+ print(f"Progress update: {message}")
+ self.assertNotIn("Progres tester", message)
+ update_found = True
+
+ self.assertTrue(start_found)
+ self.assertTrue(update_found)
+
+ @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")
+ print(f"Progress emitter path: {progress_emitter}")
+ 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 --total -1 --seconds 1 --no-details",
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:
+ title = event["body"]["title"]
+ self.assertIn("Progress tester", title)
+ start_found = True
+ if "progressUpdate" in event_type:
+ message = event["body"]["message"]
+ # We send an update on first message to set the details, so
ignore the first update
+ if not update_found:
+ self.assertTrue(message == "", message)
+ update_found = True
+
+ self.assertTrue(start_found)
+ self.assertTrue(update_found)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 3805eb3f55879..ee7614a9e3ca5 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -462,19 +462,26 @@ void ProgressEventThreadFunction(DAP &dap) {
const uint64_t total = GetUintFromStructuredData(data, "total");
const std::string details =
GetStringFromStructuredData(data, "details");
- std::string message;
- // Include the title on the first event.
- if (completed == 0) {
- const std::string title = GetStringFromStructuredData(data, "title");
- message += title;
- message += ": ";
- }
- message += details;
- // Verbose check, but we get -1 for the uint64 on failure to read
- // so we check everything before broadcasting an event.
- if (message.length() > 0)
+ // If we're only creating and sending on progress event, we send
+ // the title and the detail as a single message.
+ if (completed == 0 && total == 1) {
+ const std::string message =
+ GetStringFromStructuredData(data, "message");
dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
+ } else if (completed == 0) {
+ // On the first event, send just the title which will be captured by
+ // DAP Then if details is not empty, send a second event with the
+ // 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
+ // We don't check if message has any contents, because we still want
+ // to broadcast a status update.
+ dap.SendProgressEvent(progress_id, details.c_str(), completed,
total);
}
}
}
>From 8616ea3d894eebc779867b4829b0f5072109e858 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 20 Feb 2025 09:35:54 -0800
Subject: [PATCH 7/9] Test refactor and comment improvements based on Greg's
feedback
---
.../lldb-dap/progress/Progress_emitter.py | 9 ++--
.../lldb-dap/progress/TestDAP_Progress.py | 6 +--
lldb/tools/lldb-dap/lldb-dap.cpp | 46 ++++++++++++-------
3 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
index e962b1a10d402..9851600519e97 100644
--- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -39,8 +39,9 @@ def create_options(cls):
parser.add_option(
"--total",
dest="total",
- help="Total to count up, use -1 to identify as indeterminate",
+ help="Total to count up, use None to identify as indeterminate",
type="int",
+ default=None,
)
parser.add_option(
@@ -78,16 +79,16 @@ def __call__(self, debugger, command, exe_ctx, result):
return
total = cmd_options.total
- if total == -1:
+ if total is None:
progress = lldb.SBProgress(
"Progress tester", "Indeterminate Detail", debugger
)
else:
progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
- # Check to see if total is set to -1 to indicate an indeterminate
progress
+ # Check to see if total is set to None to indicate an indeterminate
progress
# then default to 10 steps.
- if total == -1:
+ if total is None:
total = 10
for i in range(1, total):
diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
index 2611df042f677..c7579e8074c1d 100755
--- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -109,9 +109,7 @@ def test_output_indeterminate(self):
self.dap_server.request_evaluate(
f"`command script import {progress_emitter}", context="repl"
)
- self.dap_server.request_evaluate(
- "`test-progress --total -1 --seconds 1", context="repl"
- )
+ self.dap_server.request_evaluate("`test-progress --seconds 1",
context="repl")
self.dap_server.wait_for_event("progressEnd", 15)
# Expect at least a start, an update, and end event
@@ -151,7 +149,7 @@ def test_output_nodetails_indeterminate(self):
)
self.dap_server.request_evaluate(
- "`test-progress --total -1 --seconds 1 --no-details",
context="repl"
+ "`test-progress --seconds 1 --no-details", context="repl"
)
self.dap_server.wait_for_event("progressEnd", 15)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ee7614a9e3ca5..e1f980ff4d8dc 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -463,25 +463,37 @@ void ProgressEventThreadFunction(DAP &dap) {
const std::string details =
GetStringFromStructuredData(data, "details");
- // If we're only creating and sending on progress event, we send
- // the title and the detail as a single message.
- if (completed == 0 && total == 1) {
- const std::string message =
- GetStringFromStructuredData(data, "message");
- dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
- } else if (completed == 0) {
- // On the first event, send just the title which will be captured by
- // DAP Then if details is not empty, send a second event with the
- // 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,
+ if (completed == 0) {
+ if (total == 1) {
+ // 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
- // We don't check if message has any contents, because we still want
- // to broadcast a status update.
+ } 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);
+ }
}
}
}
>From 02dcf893c3b777acc3b5989c421fda0df3596bba Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 20 Feb 2025 13:49:58 -0800
Subject: [PATCH 8/9] Fix tests, correct for new behavior
---
.../API/tools/lldb-dap/progress/Progress_emitter.py | 5 +++--
.../API/tools/lldb-dap/progress/TestDAP_Progress.py | 12 ++++++------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
index 9851600519e97..08f2d86e78d67 100644
--- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -56,6 +56,7 @@ def create_options(cls):
dest="no_details",
help="Do not display details",
action="store_true",
+ default=False,
)
return parser
@@ -81,10 +82,10 @@ def __call__(self, debugger, command, exe_ctx, result):
total = cmd_options.total
if total is None:
progress = lldb.SBProgress(
- "Progress tester", "Indeterminate Detail", debugger
+ "Progress tester", "Initial Indeterminate Detail", debugger
)
else:
- progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
+ 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.
diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
index c7579e8074c1d..d4f4f42330546 100755
--- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -87,9 +87,7 @@ def test_output_nodetails(self):
start_found = True
if "progressUpdate" in event_type:
message = event["body"]["message"]
- # We send an update on first message to set the details, so
ignore the first update
- if not update_found:
- self.assertTrue(message == "", message)
+ self.assertEqual("Initial Detail", message)
update_found = True
self.assertTrue(start_found)
@@ -128,7 +126,9 @@ def test_output_indeterminate(self):
if "progressUpdate" in event_type:
message = event["body"]["message"]
print(f"Progress update: {message}")
- self.assertNotIn("Progres tester", message)
+ # Check on the first update we set the initial detail.
+ if not update_found:
+ self.assertEqual("Step 1", message)
update_found = True
self.assertTrue(start_found)
@@ -168,9 +168,9 @@ def test_output_nodetails_indeterminate(self):
start_found = True
if "progressUpdate" in event_type:
message = event["body"]["message"]
- # We send an update on first message to set the details, so
ignore the first update
+ # Check on the first update we set the initial detail.
if not update_found:
- self.assertTrue(message == "", message)
+ self.assertEqual("Initial Indeterminate Detail", message)
update_found = True
self.assertTrue(start_found)
>From 126b3a4f7d0bc75975ccdf090ef88d12652731a8 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 20 Feb 2025 15:09:47 -0800
Subject: [PATCH 9/9] Python formatting
---
lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
index 08f2d86e78d67..1f6810d7bd7b4 100644
--- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -85,7 +85,9 @@ def __call__(self, debugger, command, exe_ctx, result):
"Progress tester", "Initial Indeterminate Detail", debugger
)
else:
- progress = lldb.SBProgress("Progress tester", "Initial Detail",
total, debugger)
+ 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.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
