Server: Remove log notification for printDebugInformation command
#16617
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For context, the initial implementation started out by sending a log notification to the client to include this information in the client channel. This is a bit ineffective because it doesn't allow the client to display this information in a more obvious way. In addition to that, it isn't obvious from a users perspective as to where the information is being printed unless they actually open the output channel. The change was to actually return this formatted string that contains the information and let the client handle how it should display this information. For example, in the Ruff VS Code extension we open a split window and show this information which is similar to what rust-analyzer does. The notification request was kept as a precaution in case there are users who are actually utilizing this way. If they exists, it should a minority as it requires the user to actually dive into the code to understand how to hook into this notification. With 0.10, we're removing the old way as it only clobbers the output channel with a long message.
|
MichaReiser
approved these changes
Mar 11, 2025
There was a problem hiding this comment.
Thank you
As a note for myself for the changelog.
This is specific to the debug command. Before, the command used to log the information. Now, the command opens a new window in VS code. Other LSP clients would require their own custom handling for this command (e.g. they could just dump the JSON output)
It might be useful to use the term "string output" instead of JSON if this is a user facing information. |
MichaReiser
pushed a commit
that referenced
this pull request
Mar 13, 2025
…16617) ## Summary For context, the initial implementation started out by sending a log notification to the client to include this information in the client channel. This is a bit ineffective because it doesn't allow the client to display this information in a more obvious way. In addition to that, it isn't obvious from a users perspective as to where the information is being printed unless they actually open the output channel. The change was to actually return this formatted string that contains the information and let the client handle how it should display this information. For example, in the Ruff VS Code extension we open a split window and show this information which is similar to what rust-analyzer does. The notification request was kept as a precaution in case there are users who are actually utilizing this way. If they exists, it should a minority as it requires the user to actually dive into the code to understand how to hook into this notification. With 0.10, we're removing the old way as it only clobbers the output channel with a long message. fixes: #16225 ## Test Plan Tested it out locally that the information is not being logged to the output channel of VS Code.
MichaReiser
pushed a commit
that referenced
this pull request
Mar 13, 2025
…16617) ## Summary For context, the initial implementation started out by sending a log notification to the client to include this information in the client channel. This is a bit ineffective because it doesn't allow the client to display this information in a more obvious way. In addition to that, it isn't obvious from a users perspective as to where the information is being printed unless they actually open the output channel. The change was to actually return this formatted string that contains the information and let the client handle how it should display this information. For example, in the Ruff VS Code extension we open a split window and show this information which is similar to what rust-analyzer does. The notification request was kept as a precaution in case there are users who are actually utilizing this way. If they exists, it should a minority as it requires the user to actually dive into the code to understand how to hook into this notification. With 0.10, we're removing the old way as it only clobbers the output channel with a long message. fixes: #16225 ## Test Plan Tested it out locally that the information is not being logged to the output channel of VS Code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
For context, the initial implementation started out by sending a log
notification to the client to include this information in the client
channel. This is a bit ineffective because it doesn't allow the client
to display this information in a more obvious way. In addition to that,
it isn't obvious from a users perspective as to where the information is
being printed unless they actually open the output channel.
The change was to actually return this formatted string that contains
the information and let the client handle how it should display this
information. For example, in the Ruff VS Code extension we open a split
window and show this information which is similar to what rust-analyzer
does.
The notification request was kept as a precaution in case there are
users who are actually utilizing this way. If they exists, it should a
minority as it requires the user to actually dive into the code to
understand how to hook into this notification. With 0.10, we're removing
the old way as it only clobbers the output channel with a long message.
fixes: #16225
Test Plan
Tested it out locally that the information is not being logged to the
output channel of VS Code.