[lldb-dap] Add an introductory message on startup.#170795
Conversation
This adds an introductory message to try to inform users on how the debug console works and adds a little more information on the current target/process, similiar to the lldb driver. Here is an example of the introduction: ``` To get started with the lldb-dap debug console try "<variable>", "help [<cmd-name>]", or "apropos <search-word>". For more information visit https://github.com/llvm/llvm-project/blob/main/lldb/tools/lldb-dap/README.md Executable binary set to 'a.out' (arm64-apple-macosx15.0.0). Attached to process 1234. ``` We may want to change the URL for more information to a page under lldb.llvm.org but that does not yet exist.
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesThis adds an introductory message to try to inform users on how the debug console works and adds a little more information on the current target/process, similiar to the lldb driver. Here is an example of the introduction: We may want to change the URL for more information to a page under lldb.llvm.org but that does not yet exist. Full diff: https://github.com/llvm/llvm-project/pull/170795.diff 3 Files Affected:
diff --git a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
index 1bfe7b7f6ef5c..5ef44cba4ebcc 100644
--- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp
@@ -50,6 +50,8 @@ ConfigurationDoneRequestHandler::Run(const ConfigurationDoneArguments &) const {
/// lldb-dap specific editor extension.
SendExtraCapabilities(dap);
+ PrintIntroductionMessage();
+
// Clients can request a baseline of currently existing threads after
// we acknowledge the configurationDone request.
// Client requests the baseline of currently existing threads after
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index d67437ad5b3ae..3841db45eae8b 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -17,6 +17,7 @@
#include "RunInTerminal.h"
#include "lldb/API/SBDefines.h"
#include "lldb/API/SBEnvironment.h"
+#include "lldb/API/SBStream.h"
#include "llvm/Support/Error.h"
#include <mutex>
@@ -261,6 +262,26 @@ void BaseRequestHandler::PrintWelcomeMessage() const {
#endif
}
+void BaseRequestHandler::PrintIntroductionMessage() const {
+ lldb::SBStream msg;
+ msg.Print("To get started with the lldb-dap debug console try "
+ "\"<variable>\", \"help [<cmd-name>]\", or \"apropos "
+ "<search-word>\".\r\nFor more information visit "
+ "https://github.com/llvm/llvm-project/blob/main/lldb/tools/"
+ "lldb-dap/README.md\r\n");
+ if (dap.target && dap.target.GetExecutable()) {
+ char path[PATH_MAX] = {0};
+ dap.target.GetExecutable().GetPath(path, sizeof(path));
+ msg.Printf("Executable binary set to '%s' (%s).\r\n", path,
+ dap.target.GetTriple());
+ }
+ if (dap.target.GetProcess()) {
+ msg.Printf("Attached to process %llu.\r\n",
+ dap.target.GetProcess().GetProcessID());
+ }
+ dap.SendOutput(OutputType::Console, {msg.GetData(), msg.GetSize()});
+}
+
bool BaseRequestHandler::HasInstructionGranularity(
const llvm::json::Object &arguments) const {
if (std::optional<llvm::StringRef> value = arguments.getString("granularity"))
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 5d235352b7738..c0b1722d26a9b 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -64,6 +64,10 @@ class BaseRequestHandler {
/// LLDB_DAP_WELCOME_MESSAGE is defined.
void PrintWelcomeMessage() const;
+ /// Prints an introduction to the debug console and information about the
+ /// debug session.
+ void PrintIntroductionMessage() const;
+
// Takes a LaunchRequest object and launches the process, also handling
// runInTerminal if applicable. It doesn't do any of the additional
// initialization and bookkeeping stuff that is needed for `request_launch`.
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
| msg.Print("To get started with the lldb-dap debug console try " | ||
| "\"<variable>\", \"help [<cmd-name>]\", or \"apropos " | ||
| "<search-word>\".\r\nFor more information visit " | ||
| "https://github.com/llvm/llvm-project/blob/main/lldb/tools/" | ||
| "lldb-dap/README.md\r\n"); |
There was a problem hiding this comment.
We have a preference that specifies if the debug console messages need to be prefixed to become LLDB commands, I believe it defaults to ` by default. Can we detect the current setting and then adjust this message to match?
There was a problem hiding this comment.
With the repl mode set to auto (the default) we should generally be able to detect commands without needing the escape character, but I'll update make a note about that.
…unction and switched to using llvm::raw_ostream instead of lldb::SBStream.
|
Looks good to me. I will let others comment to ensure they are ok with it! |
…come message about the debug console and adding a `LLDB_DAP_README_URL` in case a custom build of lldb-dap has its own URL for finding help.
|
The more I look at it seems we need to advertise the repl mode. As there is no other way for the user to know there is a repl-mode except reading the docs. something similar to. applies to other repl-modes. We can then add more information or examples in the That way the intro message does not look spammy, every time we launch dap. |
If we do detect a case where there is a variable and a command that overlap we issue a warning llvm-project/lldb/tools/lldb-dap/DAP.cpp Lines 638 to 644 in 87bf5ee I think if we include a link that has more information that should be enough and since this is on every startup, I'm trying to be brief as to not spam to much. With that in mind, I did try to shorten this some so it now prints: |
JDevlieghere
left a comment
There was a problem hiding this comment.
I think this is great for newcomers and makes the tool more accessible. However I do have some concerns about linking to the README. They're mostly orthogonal, but this puts them on the critical path, so that's why I'm bringing them up now.
- I don't think we should link to the Github repo, and instead publish documentation for
lldb-dapon https://lldb.llvm.org and link there. - The README we have today is really the documentation for the VS Code extension, rather than for our implementation of the debug adapter.
- We have this page on the website. It means well, but it's aimed at us as developers and also focuses primarily on the extension. There's nothing wrong with that, except that it's the second hit on Google when you search for "LLDB DAP" but doesn't provide any value to our users. The website is a natural place for our users to go look for documentation on how to use our tools.
Here's what I think we should do:
- Break up the current README into documentation of the debug adapter and the VS Code extension.
- Publish the documentation for the adapter on lldb.llvm.org and link back to it from the README and the welcome message.
As a stretch goal, it would be nice if we could move everything related to the extension in its own subdirectory under lldb-dap.
I'm not trying to sign you up to do the work, but that's the direction I'd like to take. If folks agree I'd be happy to file an issue to track this.
I agree, I wasn't sure where else to link for user facing documentation though, but if we publish something to lldb.llvm.org then that would be very helpful.
These sound good to me. Moving the extension code into its own subdirectory (or another folder in general) would be good as well. |
|
Would you like me to hold off on submitting this until we can make changes to the lldb.llvm.org website? Or we could update the URL once we've made those changes. |
I would prefer improving the documentation first. Mostly as a forcing function for ourselves to improve the docs, but also to make sure the "wrong" URL doesn't make it into the LLVM 22 release which branches mid-January. If you want to merge this PR before then, we could add a redirect to the https://github.com/llvm/llvm-project/blob/main/lldb/docs/.htaccess (and make sure it's a 307/Temporary Redirect). That way we can put the correct URL in the binary while we work on the improved docs. |
|
I'll make a separate PR for updating the docs and we can get that in first |
|
Created #172580 for improving the documentation on the lldb website. |
|
With #172580 submitted, I've updated the doc link to point to the updated documentation. |
This adds an introductory message to try to inform users on how the debug console works and adds a little more information on the current target/process, similiar to the lldb driver. Here is an example of the introduction: ``` To get started with the debug console try "<variable>", "<lldb-cmd>" or "help [<lldb-cmd>]". For more information visit https://lldb.llvm.org/use/lldbdap.html#debug-console. Executable binary set to 'a.out' (arm64-apple-macosx15.0.0). Attached to process 1234. ```
This adds an introductory message to try to inform users on how the debug console works and adds a little more information on the current target/process, similiar to the lldb driver. Here is an example of the introduction: ``` To get started with the debug console try "<variable>", "<lldb-cmd>" or "help [<lldb-cmd>]". For more information visit https://lldb.llvm.org/use/lldbdap.html#debug-console. Executable binary set to 'a.out' (arm64-apple-macosx15.0.0). Attached to process 1234. ```
This adds an introductory message to try to inform users on how the debug console works and adds a little more information on the current target/process, similiar to the lldb driver.
Here is an example of the introduction:
We may want to change the URL for more information to a page under lldb.llvm.org but that does not yet exist.