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

Support 'go to def' and interacting with inlay hint components #47693

Closed
mjbvz opened this issue Feb 2, 2022 · 6 comments
Closed

Support 'go to def' and interacting with inlay hint components #47693

mjbvz opened this issue Feb 2, 2022 · 6 comments
Assignees
Labels
Domain: Inlay Hints Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Domain: Type Display Bugs relating to showing types in Quick Info/Tooltips, Signature Help, or Completion Info Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Feb 2, 2022

TypeScript Version: 4.6

Search Terms

  • inlay hints
  • provideInlayHints

VS Code recently updated our inlay hints to support tagging ranges within each hint with a location (file + position). This lets us perform operations such as go to definition on individual symbols within an inlay hint

You can see the VS Code side of this API here: https://github.com/microsoft/vscode/blob/516bef8deca4a3dc8544de536eb16d5e88ad8a83/src/vscode-dts/vscode.proposed.inlayHints.d.ts#L40

Proposal

I propose that TS also starts returning symbol locations in its inlay hint response. This would let users hover and click on parts of an inlay hint to interact with it

Here's what the protocol change would look like:

interface InlayHintItem {
     // Today this is just a string
     text: string | readonly InlayHintDisplayPart[]:
  
     ....
}

interface InlayHintDisplayPart {
   // Displayed text in the ui
   text: string;

    // Definition of the symbol. 
    // TODO: Does this need to be an array or do we always have a single primary location we can use?
    span?: FileSpan;
}

As an example, consider the TS:

class A { }
class B { }

declare const Foo: A | [B];

const a = Foo;

With variable type inlay hints enabled, we currently see:

Screen Shot 2022-02-01 at 6 02 10 PM

With the new proposal, we would want to return unique InlayHintDisplayPart elements for A and B. The response would look something like:

[Trace  - 02:03:33.592] <semantic> Response received: provideInlayHints (3969). Request took 2 ms. Success: true 
Result: [
    {
        "text": [
            {
                "text": ": "
            }, {
                "text": "A",
                "span": {
                    "file": "main.ts",
                    "start": { line: 1, column: 1 }, // made up location
                    "end": { line: 1, column: 99 },
                }
            }, {
                "text": " | ["
            }, {
                "text": "B",
                "span": {
                    "file": "main.ts",
                    "start": { line: 2, column: 1 },
                    "end": { line: 2, column: 99 },
                }
            }, {
                "text": "]"
            },
        ],
        "position": {
            "line": 6,
            "offset": 8
        },
        "kind": "Type",
        "whitespaceBefore": true
    }
]

/cc @jrieken

@mjbvz
Copy link
Contributor Author

mjbvz commented Feb 2, 2022

Adding some notes from our previous discussion

How expensive would this be to compute?

On the TS side, it sounds like we should already have the symbol locations when we compute the inlay hint text

The other things is that inlay hints are only returned for the current viewport, so even in a huge file we should not be requesting thousands of these all at once

Could this apply to other symbols displayed in the UI?

Potentially. Both hovers and error text are good candidates for this

However we are not sure if it makes sense to have a unique type for each use case or if we should try having a single SymbolDisplayPartWithSpan type that covers all of these use cases

Does this need to be opt-in?

Yes, since we have already shipped inlay hints support in TS, clients would need to opt-in to start receiving InlayHintDisplayPart instead of string.

@mjbvz
Copy link
Contributor Author

mjbvz commented Feb 2, 2022

@jrieken Once question I ran into while sketching out the TS api: do we need to support multiple locations for a given span? Here's a simple TS example:

interface Foo { 
    a: number
}

interface Foo { 
    b: string
}

declare const foo: Foo;

Go to def on the type Foo currently returns two definitions. I believe this is correct, however our current proposed api only supports a single Location for each InlayHintLabelPart.

Should we change location to be location: Location | readonly Location[] | undefined?

@RyanCavanaugh RyanCavanaugh added Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript labels Feb 3, 2022
@jrieken
Copy link
Member

jrieken commented Feb 3, 2022

Go to def on the type Foo currently returns two definitions. I believe this is correct, however our current proposed api only supports a single Location for each InlayHintLabelPart.

I initially wanted to make it N locations but it doesn't have to be because we don't use the location directly but like a "cursor location", e.g it is like hovering over the first Foo, than the language brain needs to figure out what locations to merge, same for go-to-def etc. The mental model should be: user triggered language feature with a simple position and not complete definitions of the symbol

@MariaSolOs
Copy link
Contributor

@DanielRosenwasser this looks like the thing we saw in today's AUA.

@DanielRosenwasser
Copy link
Member

Should we close this?

@MariaSolOs
Copy link
Contributor

Should we close this?

I think we should (I might be biased)

@DanielRosenwasser DanielRosenwasser added this to the TypeScript 5.3.0 milestone Dec 4, 2023
@DanielRosenwasser DanielRosenwasser added Domain: Type Display Bugs relating to showing types in Quick Info/Tooltips, Signature Help, or Completion Info Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Domain: Inlay Hints labels Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Inlay Hints Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Domain: Type Display Bugs relating to showing types in Quick Info/Tooltips, Signature Help, or Completion Info Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants