Skip to content
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

debug: support location references for variable values #225546

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

connor4312
Copy link
Member

Adopts microsoft/debug-adapter-protocol#343

Also tackles some debt around debug linkification, resolving an issue
where links had native hovers that conflicted with the rich hovers added
a few iterations ago. It still uses native hovers for links in some
contexts (plain text output in the debug console) because we don't pass
through element disposables there, but it uses workbench hovers when
there's not another rich hover being used (rich debug console output).

Closes #225425

Adopts microsoft/debug-adapter-protocol#343

Also tackles some debt around debug linkification, resolving an issue
where links had native hovers that conflicted with the rich hovers added
a few iterations ago. It still uses native hovers for links in some
contexts (plain text output in the debug console) because we don't pass
through element disposables there, but it uses workbench hovers when
there's not another rich hover being used (rich debug console output).

![](https://memes.peet.io/img/24-08-8aacba27-0903-4313-b7f6-6b3bbbff6970.png)

Closes #225425
@connor4312 connor4312 self-assigned this Aug 13, 2024
@connor4312 connor4312 enabled auto-merge (squash) August 13, 2024 23:08
@vs-code-engineering vs-code-engineering bot added this to the August 2024 milestone Aug 13, 2024
@connor4312 connor4312 merged commit 676058f into main Aug 13, 2024
6 checks passed
@connor4312 connor4312 deleted the connor4312/issue225425 branch August 13, 2024 23:27

const location = await this.raw.locations({ locationReference });
if (!location?.body) {
throw new Error(localize('noDebugAdapter', "No debugger available, can not send '{0}'", 'locations'));

Choose a reason for hiding this comment

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

incorrect error message?

@vogelsgesang
Copy link

vogelsgesang commented Aug 15, 2024

Could it be that this does not work for remote debugging sessions (using the Remote SSH extension), yet?

I am currently trying to implement this in the lldb-dap debug adapter, but when clicking the link in the "variables" view, I am getting the error "The editor could not be opened because the file was not found".

image

Towards the bottom of the screenshot, you can see the reply of the debug adapter. Note how it refers to the file as /home/avogelsgesang/Documents/corotest/test-ptr.cpp. This is the exact same path which is also used by the debug adapter in responses to stackTrace commands, and for stack traces this path is correctly linked.

In a couple of other requests, I am seeing VS-Code to refer to this path as vscode-remote://ssh-remote%2Bavogelsges-wsl2/home/avogelsgesang/Documents/corotest/test-ptr.cpp. Could it be that some path mapping for remote files is still missing here?

vogelsgesang added a commit to vogelsgesang/llvm-project that referenced this pull request Aug 16, 2024
This commit implements support for the "declaration location" recently
added by microsoft/debug-adapter-protocol#494 to the debug adapter
protocol.

For the `declarationLocationReference` we need a variable ID similar to
the the `variablesReference`. I decided to simply reuse the
`variablesReference` here and renamed `Variables::expandable_variables`
and friends accordingly. Given that almost all variables have a
declaration location, we now assign those variable ids to all variables.

While `declarationLocationReference` effectively supersedes
`$__lldb_extensions.declaration`, I did not remove this extension, yet,
since I assume that there are some closed-source extensions which rely
on it.

I tested this against VS-Code Insiders. However, VS-Code Insiders
currently only supports `valueLoctionReference` and not
`declarationLocationReference`, yet. Locally, I hence tried to publish
the declaration locations as value locations. However, it seems that
VS-Code still has issues with correctly resolving file paths, as
reported in microsoft/vscode#225546 (comment)
vogelsgesang added a commit to vogelsgesang/llvm-project that referenced this pull request Aug 16, 2024
This commit implements support for the "declaration location" recently
added by microsoft/debug-adapter-protocol#494 to the debug adapter
protocol.

For the `declarationLocationReference` we need a variable ID similar to
the the `variablesReference`. I decided to simply reuse the
`variablesReference` here and renamed `Variables::expandable_variables`
and friends accordingly. Given that almost all variables have a
declaration location, we now assign those variable ids to all variables.

While `declarationLocationReference` effectively supersedes
`$__lldb_extensions.declaration`, I did not remove this extension, yet,
since I assume that there are some closed-source extensions which rely
on it.

I tested this against VS-Code Insiders. However, VS-Code Insiders
currently only supports `valueLoctionReference` and not
`declarationLocationReference`, yet. Locally, I hence tried to publish
the declaration locations as value locations. However, it seems that
VS-Code still has issues with correctly resolving file paths, as
reported in microsoft/vscode#225546 (comment)
vogelsgesang added a commit to vogelsgesang/llvm-project that referenced this pull request Aug 16, 2024
This commit implements support for the "declaration location" recently
added by microsoft/debug-adapter-protocol#494 to the debug adapter
protocol.

For the `declarationLocationReference` we need a variable ID similar to
the the `variablesReference`. I decided to simply reuse the
`variablesReference` here and renamed `Variables::expandable_variables`
and friends accordingly. Given that almost all variables have a
declaration location, we now assign those variable ids to all variables.

While `declarationLocationReference` effectively supersedes
`$__lldb_extensions.declaration`, I did not remove this extension, yet,
since I assume that there are some closed-source extensions which rely
on it.

I tested this against VS-Code Insiders. However, VS-Code Insiders
currently only supports `valueLoctionReference` and not
`declarationLocationReference`, yet. Locally, I hence tried to publish
the declaration locations as value locations. However, it seems that
VS-Code still has issues with correctly resolving file paths, as
reported in microsoft/vscode#225546 (comment)
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Sep 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support debug location references
3 participants