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

Add Inline value provider api for debugging #43449

Open
mjbvz opened this issue Mar 30, 2021 · 26 comments
Open

Add Inline value provider api for debugging #43449

mjbvz opened this issue Mar 30, 2021 · 26 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Mar 30, 2021

From microsoft/vscode#119489

Background

The new inline value api for VS Code lets language extensions tell the debugger which values should be shown directly in a file while debugging. Without this API, VS Code uses a generic method for determining which values to show. This ends up showing many irrelevant values to the user

See microsoft/vscode#119489 for an example of this

Feature Request

We'd like to use the TypeScript server's language smarts to implement the VS Code inline value api so that only relevant values are shown.

Here's a potential shape of the API based on the vscode inline value API

// For debugging, we request inline values at a specific location in a file
interface InlineValueRequestArgs extends FileLocationRequestArgs {}

interface InlineValueResponse  extends Response {
   body: InlineValue[];
}

// Support two types of inline values: simple ones that can be looked up by name, and more complex ones
// that require evaluating an expression
type InlineValue = InlineValueVariableLookup | InlineValueEvaluatableExpression;

// A value that can be looked up by name.
// This is mostly just an optimization for a common case of inline values 
interface InlineValueVariableLookup {
    // Span in code of the identifier
    readonly span: TextSpan;

    // Optional variable name to look up. If not specified, uses the name from `span`
    // TODO: Is this needed for TS?
    readonly variableName?: string;
}

// A value which is shown by evaluating a given expression
interface InlineValueEvaluatableExpression {
    // Span in code of the expression
    readonly span: TextSpan;

    // Optional expression to evaluate. If not specified, uses the expression from `span`
    // TODO: Is this needed for TS?
    readonly expression?: string;
}

Behavior

Some notes on how the inline value provider should work (copied from @connor4312):

  • The debugger should handle source maps, so the provider should only have to worry about the requested file

  • Inline values should only be provided in the current and parent scopes

  • Declarations and assignments assignments should evaluate the declared or assigned expression, bar in bar = foo() or foo in function foo() {}

  • Conditional expressions (ternary or if statements) should evaluate the conditional and none of its children, !foo && bar in if (!foo && bar)

  • Property assignments should be evaluated, foo() in { x: foo() }

Here's a small example of some inline values:

function double(n: number) { // eval('double') -> double()
  return n * 2;
}

let x = 1; // InlineValueVariableLookup for x here

if (x === 2) { // eval(x === 2) -> false
  x *= 2; // this line should NOT be evaluated since it's not the scope or its parents
}

if (x === 1) { // eval(x === 1) -> true
  x *= 2; // InlineValueVariableLookup for x here
  const y = {
    z: x ** 2, // eval(x ** 2) -> 16
  };

  console.log(x); // <-- paused here
}

@connor4312 Is handling this on the VS Code side and should be able to answer any questions regarding this API

/cc @weinand

@mjbvz
Copy link
Contributor Author

mjbvz commented Mar 31, 2021

A few notes from our discussion:

Should this command really live in TS core?

Pros:

  • We do need some language smarts to handle all cases
  • Fits with other apis
  • Don't double parse AST
  • VS Code doesn't have to worry about AST versioning

Cons:

  • TS team doesn't maintain debugger
  • Only single client right now (VS Code)

Could TS implement a more generic API instead?

Instead of offering an InlineValueProvider, maybe TS could expose apis for determining scopes and resolving values. VS Code could then use this to implement an inline value provider

Some existing apis that are similar:

  • Folding
  • Selection ranges

Could we implement this with a TS Server plugin?

Potentially, and this would also potentially make it easier for the VS Code team to maintain

However there is currently no way to add a new command through a plugin. Could we repurpose an existing command instead?

@mjbvz
Copy link
Contributor Author

mjbvz commented Mar 31, 2021

@connor4312 The TS team is interested in supporting this feature, but understandably would like to avoid maintaining an API whose expected behavior is tied so closely to the debugger

This leaves a few options:

  • Come up with some more generic API building blocks that we could use to implement inline values.

    Are the any other debugger APIs that are similar?

  • Implement inline values entirely in VS Code

  • Implement in TypeScript but with a component that VS Code maintains

  • Implement using a language service plugin

The language service plugin seems like it may be the most interesting, although we'd potentially need to do some work on the TS side to support a new command from a plugin

@sheetalkamat
Copy link
Member

Allowing plugins to enable new commands should be doable if we can pass session to the enablePlugin apart from Project. Session already has addProtocolHandler that can be leveraged

@connor4312
Copy link
Member

Are the any other debugger APIs that are similar?

The evaluatable expression provider is vaguely similar in that it is a debugger behavior that's driven by language semantics. Though VS Code has a built-in provider that has good enough heuristics for C-style language like J/TS.

I don't have enough knowledge around the architecture of language servers and services to be able to provide an architectural recommendation.

@weinand
Copy link

weinand commented Apr 1, 2021

@mjbvz you said:

Instead of offering an InlineValueProvider, maybe TS could expose apis for determining scopes and resolving values. VS Code could then use this to implement an inline value provider

This would be my preferred approach.

Most likely this API could be used for solving a similar set of problems that we have for the debug hover (where our current heuristics is not able to resolve variables to the correct scope). The "evaluatable expression provider" is basically a twin to the "Inline values provider" and could solve the problems by using the same language server APIs.

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Apr 1, 2021
@weinand
Copy link

weinand commented Apr 12, 2021

@mjbvz what is the status of this feature request?
Is the TS team expecting a proposal for your "api for determining scopes and resolving values"?

@mjbvz
Copy link
Contributor Author

mjbvz commented Apr 12, 2021

@weinand We are currently late in the TS 4.3 release so I think this would have be scheduled for TS 4.4, which I believe would start development in May and be released in July

@RyanCavanaugh / @DanielRosenwasser It would make more sense for @weinand to talk to someone on the TS team directly instead of going through me. Who would be the best point of contact on the TS team for @weinand to work with on this feature?

@connor4312
Copy link
Member

I am also happy to be involved in development here.

@DanielRosenwasser
Copy link
Member

I'd say me and @minestarks, but ideally we'd just have the discussion at the weekly editor sync.

@Kingwl
Copy link
Contributor

Kingwl commented May 25, 2021

Happy to work on this.

@mjbvz
Copy link
Contributor Author

mjbvz commented May 26, 2021

Following up on our discussion:

  • We're going to look into the TS language service plugin based approach. This will let us reuse the TS AST while iterating quickly on the new API
  • If there's need for this in other editors, in the future we could consider making it part of the standard TS Server api

Work items:

@Kingwl Thanks for offering to help! Once we have started writing the plugin itself, check with @connor4312 to see if he could use your help

@Kingwl
Copy link
Contributor

Kingwl commented May 27, 2021

Will investigate allowing a plugin to register a new command

That's very helpful!
May I ask @sheetalkamat started the work? If not, I'm happy to work on this too!

PLUS: I have tried about a plugin to support Inline hints preview. But I realize we need new command support.

@Kingwl
Copy link
Contributor

Kingwl commented May 27, 2021

Here's a simple try on the plugin API.

And I'm tried integrating with the InlayHints features.

image

I have some confuse:

Are there any way to communicate with tsserver in another custom extension? rather than the typescript-language-features. Or we have to fork another tsserver for our own?

@weinand
Copy link

weinand commented May 27, 2021

@Kingwl The "InlayHints" API is not what should be used for the debugger Inline Values feature.
The correct API to use is the "InlineValuesProvider" API,
please see https://code.visualstudio.com/updates/v1_54#_inline-value-provider-api

And the implementation of the "InlineValuesProvider" for TypeScript will live in the typescript-language-features extension (because I don't think that there is a way to communicate with tsserver in another extension).

@weinand
Copy link

weinand commented May 27, 2021

@Kingwl you can just use this extension as a starting point:
inlinevaluesprovider.zip

@Kingwl
Copy link
Contributor

Kingwl commented May 27, 2021

@weinand Ahhhh yep.

I'm not trying about inline value api but the tsserver's custom protocol handler...

@Kingwl
Copy link
Contributor

Kingwl commented May 27, 2021

because I don't think that there is a way to communicate with tsserver in another extension.

If so, the ts plugin & tsserver 's custom protocol handler will be very limited.(might only useful to the devs for typescript-language-features.

As I said, you have to fork your tsserver process. Which means you cannot use the one who has been used by editor.

@weinand
Copy link

weinand commented May 27, 2021

@Kingwl I do not understand this:

As I said, you have to fork your tsserver process. Which means you cannot use the one who has been used by editor.

The "InlineValueProvider" will not be part of the debugger extension but will live in the same language extension that is used by the editor. The whole idea of this effort is to avoid forking the tsserver process but reusing the tsserver that's already running.

@Kingwl
Copy link
Contributor

Kingwl commented May 27, 2021

The "InlineValueProvider" will not be part of the debugger extension but will live in the same language extension that is used by the editor.

Yep. I know that. Actually I'm not talking about the inlineValueProvider. My point is the ts plugin & tsserver 's extensibility what will be a part of this feature(If i'm correct):

If we allows the ts plugin provide their own commands, we should also could use these commands in any extensions rather than the only one builtin extension.

@sheetalkamat
Copy link
Member

I am not sure what you mean.. the plugins are suppose to use session.executeCommand or session.onMessage to run the commands.

@Kingwl
Copy link
Contributor

Kingwl commented May 31, 2021

I am not sure what you mean

Well. Sorry for the confused.

I mean currently ts plugin & tsserver can only communicate with the vscode built-in typescript extension.

If I'd like to extend some whole feature (e.g. Some vscode feature who vscode built-in typescript extension has not supported yet) by a vscode extension who I wrote.

Unfortunately, seems it's not possible to reuse the tsserver who used by vscode built-in extension. Because we cannot send some message to tsserver and get the response. (By stdin/stdout) (even session.executeCommand and session.onMessage)

I'm looking forward to there's something like 'a plugin of vscode built-in typescript extension'. Or the built-in extension provide some commands kind of forward to tsserver.

@weinand
Copy link

weinand commented May 31, 2021

@Kingwl extending the tsserver (which runs in VS Code's builtin JS/TS extension) by another extension is not possible (@mjbvz please correct me if I'm wrong).
But this is not a problem because we "own" VS Code's builtin JS/TS extension and can easily add the inline support there.

@DanielRosenwasser
Copy link
Member

We discussed this again within the team.

One concern is that it's not clear to us how inline values are leveraged by the editor and debugger, so it's hard for us to evaluate an implementation here. We don't know what kinds of implementation issues we might run into, and what constraints the debugger has.

We also feel like the behavior might change over time, so we have some concerns over whether or not we should provide this as a public part of TSServer initially.

Let's discuss these again at the next editor sync if you're all open to it.

@connor4312
Copy link
Member

connor4312 commented Aug 28, 2021

Can you forward me an invite? My alias is copeet. @weinand may also be interested. Thanks!

@weinand
Copy link

weinand commented Aug 30, 2021

My mental model for this feature request is still what was summarised by @mjbvz in #43449 (comment):

We are not asking for an Inline value API from TypeScript, but instead we (VS Code) want to implement an "Inline value provider" in VS Code's TypeScript extension. This "Inline value provider" will do things like: find all interesting variables in a given source line.
So the TypeScript extension (which is VS Code specific) functions as a bridge between the language service and the debugger.

But in order to do so, we need access to the TypeScript AST. Since there is no direct TS AST API available, we would like to implement the "Inline value provider" (or some part of it) as a "TS language service plugin" and the VS Code TypeScript extension needs a way to communicate with the "TS language service plugin".

With this approach an "Inline value provider" is not a public part of the TSServer, but it is private code of the VS Code TypeScript extension that gets injected into the TSServer at runtime. Any concerns about "how inline values are leveraged by the editor and debugger" or "what constraints the debugger has" are addressed in the TypeScript extension, and not in the TSServer.

@hediet
Copy link
Member

hediet commented Aug 30, 2021

It would be really nice if this issue could be solved by addressing this more general issue about enabling VS Code extensions to call into third party TypeScript extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants