Skip to content

[SBProgress][CLI] Configure sbprogress events to be emitted for the CLI #133309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

Jlalond
Copy link
Contributor

@Jlalond Jlalond commented Mar 27, 2025

In the original SBProgress patch, #123837, I didn't ensure the debugger was broadcasting these events to the CLI as SBProgress has far been focused on DAP. We had an internal ask to have SBProgress events broadcasted to the CLI so this patch addresses that.

image

@Jlalond Jlalond requested a review from JDevlieghere as a code owner March 27, 2025 20:21
@llvmbot llvmbot added the lldb label Mar 27, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 27, 2025

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

In the original SBProgress patch, #123837, I didn't ensure the debugger was broadcasting these events to the CLI. This patch addresses that.

<img width="387" alt="image" src="https://github.com/user-attachments/assets/5eb93a46-1db6-4d46-a6b7-2b2f9bbe71db" />


Full diff: https://github.com/llvm/llvm-project/pull/133309.diff

1 Files Affected:

  • (modified) lldb/source/Core/Debugger.cpp (+1-1)
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 8c705f889983a..cf247974ad966 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -2024,7 +2024,7 @@ lldb::thread_result_t Debugger::DefaultEventHandler() {
               }
             }
           } else if (broadcaster == &m_broadcaster) {
-            if (event_type & lldb::eBroadcastBitProgress)
+            if (event_type & lldb::eBroadcastBitProgress || event_type & lldb::eBroadcastBitExternalProgress)
               HandleProgressEvent(event_sp);
             else if (event_type & lldb::eBroadcastBitWarning)
               HandleDiagnosticEvent(event_sp);

@Jlalond Jlalond requested a review from clayborg March 27, 2025 20:21
Copy link

github-actions bot commented Mar 27, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@Jlalond Jlalond force-pushed the sbprogress-cli-events branch from d584044 to d55d7e3 Compare March 27, 2025 20:29
Copy link
Collaborator

@clayborg clayborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with this being always enabled. I will defer to Jonas to make sure he is

@Jlalond
Copy link
Contributor Author

Jlalond commented Mar 27, 2025

I am fine with this being always enabled. I will defer to Jonas to make sure he is

So @JDevlieghere and I agreed on adding this to the default listener which is currently the case. But because we only tested DAP after we added the new broadcast bit, this fell through the cracks!

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Jlalond Jlalond merged commit 2a96bec into llvm:main Mar 27, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants