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

Investigate into using language service support for scope resolution #84044

Open
weinand opened this issue Nov 6, 2019 · 8 comments
Open

Investigate into using language service support for scope resolution #84044

weinand opened this issue Nov 6, 2019 · 8 comments
Assignees
Labels
Milestone

Comments

@weinand
Copy link
Member

@weinand weinand commented Nov 6, 2019

Related Issues:

  • Debug hover does not use AST #24520
  • Debug Hover Box don't show up even in the right scope #37838
  • Inline values are shown for a variable in different scope if it has same name in current scope #19215

Examples of bogus behavior:

2019-11-08_16-58-00 (1)

@weinand weinand added this to the November 2019 milestone Nov 6, 2019
@weinand weinand self-assigned this Nov 6, 2019
@kieferrm kieferrm mentioned this issue Nov 11, 2019
50 of 68 tasks complete
@weinand

This comment has been minimized.

Copy link
Member Author

@weinand weinand commented Dec 2, 2019

VS Code's hover functionality suffers from some problems:

  • if different variables have the same name in different scopes, VS Code does not always show the correct value.
  • if the source language is not the same as the target language, hovering over variables does not show a value at all.
  • hovering over a complex variable path (e.g. "doc->foo[123]") does not show correct values or values at all.

These issues are a consequence of VS Code's generic hover implementation and insufficient implementations of specific debug extension/adapters.

VS Code's generic hover implementation makes use of some language agnostic heuristics:

  • VS Code uses a simple regular expression to find the hovered expression under the mouse. E.g. the index operator "[123]" in "doc->foo[123]" is not detected. In addition this regular expression is not contributed by the source language.
  • When hovering over source text, most variables detected via the regular expression from above are actually "false positives" because they are "out of scope" with respect to the current execution context (stackframe). In order to avoid showing hovers for them, VS Code uses the source code ranges in DAP's StackFrame and Scope objects. Since this range information is of bad quality, VS Code ignores them for hovers but uses them when showing inline values (this results in the problem shown in the initial comment from above).
  • depending on some capability of the debug adapter VS Code uses one of two strategies for finding the value of a variable under the mouse:
    • the variable name is matched against the variables in VS Code's VARIABLES view.
    • the variable name is used as an expression that is evaluated via the debug adapter's "evaluate" request.

I propose the following improvements:

  • introduce new API for finding the debug specific expression for a given source location (= hover location). VS Code would use this API in favour of its builtin regular expression. At the moment it is not clear whether this API lives on a language extension or on the DAP. In the latter case the debug adapter would need new (reverse) DAP API to delegate this request back to the language service in the client. The advantage of the latter approach is that the debug adapter has a chance to modify the expression returned from the language service so it can be used by an upcoming "evaluate" request.
  • introduce new (reverse) DAP API so that a debug adapter can get scope range information from its client (e.g. VS Code). The debug adapter could use this API to provide correct range information in DAP Scope and Frame objects returned to the client. This helps the client to know the correct scope of a variable.
  • introduce new (reverse) DAP API so that a debug adapter can map variable names between target and source languages. This makes it possible that a debug adapter can return DAP Variable objects with source names (instead of target names like it does today).
@weinand weinand closed this Dec 2, 2019
@weinand weinand removed the feature-request label Dec 2, 2019
@weinand

This comment has been minimized.

Copy link
Member Author

@weinand weinand commented Dec 2, 2019

Since this is an investigation there is nothing to verify or test for now.

@weinand

This comment has been minimized.

Copy link
Member Author

@weinand weinand commented Dec 11, 2019

Reopened for continuing work in Dec/Jan...

@weinand weinand reopened this Dec 11, 2019
@gjsjohnmurray

This comment has been minimized.

Copy link
Contributor

@gjsjohnmurray gjsjohnmurray commented Dec 17, 2019

introduce new API for finding the debug specific expression for a given source location (= hover location)

Please prioritize this aspect. Our debugger deals with a language where A and %A are two distinct variables. The existing behaviour displays the value of A when hovering over an occurrence of %A.

@GustavoASC

This comment has been minimized.

Copy link
Contributor

@GustavoASC GustavoASC commented Jan 21, 2020

We also need this because we are developing a COBOL debug extension. Since COBOL variables have hyphen as part of the name, this piece of code is probably making hover return only part of the variable name.

Providing an extension callback to configure hover behavior or allowing extension to specify a hover regex would be great.

Maybe I can help you, making a pull request to VSCode core, according to what you think is the best approach to develop this solution.

@weinand

This comment has been minimized.

Copy link
Member Author

@weinand weinand commented Jan 22, 2020

@GustavoASC thanks for the offer to submit a PR. But I'm already working on this new extension API.

@weinand

This comment has been minimized.

Copy link
Member Author

@weinand weinand commented Jan 22, 2020

I've created a new API request.

@weinand

This comment has been minimized.

Copy link
Member Author

@weinand weinand commented Jan 27, 2020

Work will continue in February.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.