https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/124648
>From 0587ba8239dbbd22aa40bde23d61f9504975817d 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 b64fe53741486ea9b6cde2e658b766aa68bf9389 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 96f0ebb43f83cce80c76915f5412b9bb31dd3f12 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 ++--
.../Handler/InitializeRequestHandler.cpp | 77 ++++++++++++++++---
2 files changed, 73 insertions(+), 20 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/Handler/InitializeRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index e9041f3985523..9f78349379069 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/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 == 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 {
+ // 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 07cbc9d0bd54eaaf5f3a03595e67f2b9829d8e72 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
---
.../Handler/InitializeRequestHandler.cpp | 47 +++++--------------
1 file changed, 12 insertions(+), 35 deletions(-)
diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index 9f78349379069..c3cea94bd3bf3 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -25,12 +25,8 @@ 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 + 1);
+ keyValue.GetStringValue(&str[0], length);
return str;
}
@@ -68,38 +64,19 @@ 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) {
- 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 {
- // 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);
+ 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)
+ dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
}
}
}
>From 4268576b2c1c08f4f6f1febd693699766d76bdfe Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 19 Feb 2025 12:49:20 -0800
Subject: [PATCH 5/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 +++++++++++++++++-
3 files changed, 156 insertions(+), 5 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)
>From 10a4e37bdebabd7e1bdf4d118d860fd6c658d8f6 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 20 Feb 2025 09:35:54 -0800
Subject: [PATCH 6/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 +--
.../Handler/InitializeRequestHandler.cpp | 47 ++++++++++++++-----
3 files changed, 42 insertions(+), 20 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/Handler/InitializeRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index c3cea94bd3bf3..9f78349379069 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -25,8 +25,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;
}
@@ -64,19 +68,38 @@ 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 += ": ";
+ 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 {
+ // 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);
}
-
- 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)
- dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
}
}
}
>From 318f29325d0c3fde58800d4a8f863f750f6d2704 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 20 Feb 2025 13:49:58 -0800
Subject: [PATCH 7/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 79394c3887bf2a3b745a1784a104b835c1e2cde8 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Thu, 20 Feb 2025 15:09:47 -0800
Subject: [PATCH 8/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.
>From 681352cc4f771bc1dec873f5105d6515fefc1997 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 26 Feb 2025 12:00:06 -0800
Subject: [PATCH 9/9] Refactor tests to be concise and add progresse end, add
additional check to sbprogress description check
---
lldb/source/API/SBProgress.cpp | 2 +-
.../lldb-dap/progress/TestDAP_Progress.py | 144 +++++++-----------
2 files changed, 55 insertions(+), 91 deletions(-)
diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp
index 403900aed4a8c..d40e11da973d4 100644
--- a/lldb/source/API/SBProgress.cpp
+++ b/lldb/source/API/SBProgress.cpp
@@ -41,7 +41,7 @@ void SBProgress::Increment(uint64_t amount, const char
*description) {
LLDB_INSTRUMENT_VA(amount, description);
std::optional<std::string> description_opt;
- if (description)
+ if (description && description[0])
description_opt = description;
m_opaque_up->Increment(amount, description_opt);
}
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 d4f4f42330546..968351c8179f1 100755
--- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -12,6 +12,42 @@
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"]
+ print(f"Progress update: {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")
@@ -30,28 +66,10 @@ 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:
- 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)
+ self.verify_progress_events(
+ expected_title="Progress tester",
+ expected_not_in_message="Progress tester",
+ )
@skipIfWindows
def test_output_nodetails(self):
@@ -71,27 +89,10 @@ def test_output_nodetails(self):
"`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"]
- self.assertEqual("Initial Detail", message)
- update_found = True
-
- self.assertTrue(start_found)
- self.assertTrue(update_found)
+ self.verify_progress_events(
+ expected_title="Progress tester",
+ expected_message="Initial Detail",
+ )
@skipIfWindows
def test_output_indeterminate(self):
@@ -109,30 +110,11 @@ def test_output_indeterminate(self):
)
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
- # 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}")
- # 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)
- self.assertTrue(update_found)
+ self.verify_progress_events(
+ expected_title="Progress tester",
+ expected_message="Step 1",
+ only_verify_first_update=True,
+ )
@skipIfWindows
def test_output_nodetails_indeterminate(self):
@@ -152,26 +134,8 @@ def test_output_nodetails_indeterminate(self):
"`test-progress --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"]
- # Check on the first update we set the initial detail.
- if not update_found:
- self.assertEqual("Initial Indeterminate Detail", message)
- update_found = True
-
- self.assertTrue(start_found)
- self.assertTrue(update_found)
+ self.verify_progress_events(
+ expected_title="Progress tester",
+ expected_message="Initial Indeterminate Detail",
+ only_verify_first_update=True,
+ )
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
