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

Inlay hints: Allow ctrl/cmd+click to go to a type's definition #129528

Closed
Tyriar opened this issue Jul 27, 2021 · 32 comments
Closed

Inlay hints: Allow ctrl/cmd+click to go to a type's definition #129528

Tyriar opened this issue Jul 27, 2021 · 32 comments
Assignees
Labels
api feature-request Request for new features or functionality inlay-hints on-release-notes Issue/pull request mentioned in release notes on-testplan
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jul 27, 2021

Testing #129381

I tried ctrl+clicking TerminalIcon here intuitively:

image

Going to an argument definition would also be handy/expected

@Tyriar
Copy link
Member Author

Tyriar commented Jul 27, 2021

In a similar vein, the hover for the inlays should show for the type they're hovering imo

@hediet hediet assigned mjbvz and unassigned hediet Jul 27, 2021
@hediet
Copy link
Member

hediet commented Jul 27, 2021

I don't know what the plans are for inlay hints, but I think this is doable with injected text.
However, the injected text feature needs to be extended to support hovers & decorations that just apply to a specific part of the injected text.

@mjbvz mjbvz assigned jrieken and mjbvz and unassigned mjbvz Jul 27, 2021
@mjbvz
Copy link
Collaborator

mjbvz commented Jul 27, 2021

@jrieken Adding you since I believe VS Code's inlay hints api would need to be modified to support this

@mjbvz mjbvz added this to the Backlog milestone Jul 27, 2021
@mjbvz mjbvz added the feature-request Request for new features or functionality label Jul 27, 2021
@jrieken
Copy link
Member

jrieken commented Aug 16, 2021

Yeah, we want that

@jrieken jrieken modified the milestones: Backlog, September 2021 Sep 1, 2021
@jrieken jrieken assigned hediet and unassigned mjbvz Sep 1, 2021
@jrieken
Copy link
Member

jrieken commented Sep 1, 2021

Stretch for September

@jrieken jrieken modified the milestones: September 2021, October 2021 Sep 23, 2021
@jrieken jrieken removed this from the October 2021 milestone Oct 8, 2021
@jrieken jrieken added this to the January 2022 milestone Dec 20, 2021
jrieken added a commit that referenced this issue Jan 4, 2022
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this issue Jan 4, 2022
…ion for each part/node, prep for microsoft/vscode#129528

Commit: f0cc5604d7fd0739bc10c0cc5e956699dc4fbbde
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this issue Jan 4, 2022
Commit: 1b6e853df16c5289082c63bf391f5ec62479f2ac
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this issue Jan 4, 2022
…29528

Commit: b61f32323db74cf9e4945431a835ba15190f64e0
jrieken added a commit that referenced this issue Jan 10, 2022
jrieken added a commit that referenced this issue Jan 10, 2022
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this issue Jan 10, 2022
Commit: 62bae334d5b12f02ef4078c8114336426de346e0
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this issue Jan 10, 2022
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this issue Jan 10, 2022
…n and happens only once, microsoft/vscode#129528

Commit: 1fdcfde34410ca50dcfb3e5a534323eb2d087571
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this issue Jan 10, 2022
… context menu on right click, microsoft/vscode#129528

Commit: 120718992a05ccf3cf4b90237128e7c561086997
@jrieken jrieken closed this as completed Jan 21, 2022
@jrieken jrieken added the on-release-notes Issue/pull request mentioned in release notes label Jan 28, 2022
@orta
Copy link
Contributor

orta commented Feb 6, 2022

👋 I think this change isn't backwards compatible (at Monaco level):

-	provideInlayHints(model: model.ITextModel, range: Range, token: CancellationToken): ProviderResult<InlayHint[]>;
+ 	provideInlayHints(model: model.ITextModel, range: Range, token: CancellationToken): ProviderResult<InlayHintList>;

Because you used to return an array of hints, and now you need to return the object with hints (and it crashes if you pass the array ) microsoft/TypeScript-Website#2247

( I'm happy to make changes to our usage, but would also need to find a way to know which shape of data to provide. )

@jrieken
Copy link
Member

jrieken commented Feb 7, 2022

Inlay hints are still proposed API which means breaking changes are happening. Also, there is no stability guarantee wrt API for monaco

@SomeoneToIgnore
Copy link

@jrieken
Sorry for asking in not the most appropriate place, please point me to the right issue, if any is present.

I'm reusing the proposed API in rust-analyzer and came up with a small test change: https://github.com/rust-analyzer/rust-analyzer/compare/master...SomeoneToIgnore:kb/vscode-inlay-hints?expand=1

Similar changes worked and displayed the hints on early proposal's stages, but not now:
image

with the let i = 22; line receiving the first item from the json array on the right.

I see no errors or warnings in any logs available, ensured that all experimental APIs are enabled and the extension installs and starts normally, so interested, what else have I missed?

@jrieken
Copy link
Member

jrieken commented Feb 9, 2022

Hard to distance-diagnose... Is the feature enabled? Check the "editor.inlayHints.enabled": true-setting?

Also, do "F1 > Log Level > Trace" and check the extension host log via "F1 > Show Logs > Extension Host". It should show messages "similar" to these [2022-02-09 13:39:16.135] [exthost] [trace] [vscode.typescript-language-features] INVOKE provider 'j=>j.provideInlayHints(f.URI.revive(U),N,W)'

@SomeoneToIgnore
Copy link

SomeoneToIgnore commented Feb 9, 2022

The former is on, but the latter does not happen: I see no trace entries in the logs at all.
I will check on that more, but if you happen to have any API usage example at hand (the one from the video, maybe?), that would help me drastically.

Thanks nonetheless.

@jrieken
Copy link
Member

jrieken commented Feb 9, 2022

Can you check the trace log of the window for messages like [2022-02-09 13:58:11.398] [renderer1] [trace] [DEBOUNCE: InlayHint] for file:///Users/jrieken/Code/vscode/src/vs/editor/common/services/editorSimpleWorker.ts is 40.166666666666664ms? That is a good indication if the feature has a provider it wants to work with...

The sample snippet from the video above is this. Notice how it is registered for the fooLang (which is my test/fiddle language). Changing that to rust or typescript should enable this

vscode.languages.registerInlayHintsProvider('fooLang', new class implements vscode.InlayHintsProvider {
        // onDidChangeInlayHints?: vscode.Event<void> | undefined;
        provideInlayHints(doc: vscode.TextDocument, range: vscode.Range, _token: vscode.CancellationToken) {
            const result: vscode.InlayHint[] = [];
            const text = doc.getText();

            {
                let end = doc.offsetAt(range.end);
                let pos = doc.offsetAt(range.start);
                while (pos < end) {
                    const idx = text.indexOf('bar', pos)
                    if (idx < 0) {
                        break;
                    }
                    pos = idx + 4;
                    const start = doc.positionAt(idx);
                    result.push(new vscode.InlayHint(start, [{ value: 'foo' }]))
                    // result.push(new vscode.InlayHint('FooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFooFoo', start))
                }
            }
            {
                let end = doc.offsetAt(range.end);
                let pos = doc.offsetAt(range.start);
                while (pos < end) {
                    const idx = text.indexOf('foo', pos)
                    if (idx < 0) {
                        break;
                    }
                    pos = idx + 4;
                    const start = doc.positionAt(idx + 3);
                    const label: vscode.InlayHintLabelPart[] = [
                        { value: ': ' },
                        { value: 'Map' },
                        { value: '<number, ' },
                        { value: 'typeof document' },
                        { value: '' },
                        { value: '>' },
                    ]
                    const hint = new vscode.InlayHint(start, label);
                    hint.paddingLeft = true;
                    hint.command = { command: 'extension.helloWorld', title: 'sss' }
                    result.push(hint)
                }
            }
            return result;
        }

        async resolveInlayHint(hint: vscode.InlayHint): Promise<vscode.InlayHint> {
            await new Promise(resolve => setTimeout(resolve, Math.random() * 1234))
            hint.tooltip = new vscode.MarkdownString('I am **Resolved** $(zap)', true);

            if (typeof hint.label !== 'string') {
                for (let part of hint.label) {
                    if (part.value === 'Map') {
                        part.location = new vscode.Location(
                            vscode.Uri.file('/Users/jrieken/Code/vscode/extensions/node_modules/typescript/lib/lib.es2015.collection.d.ts'),
                            new vscode.Position(20, 10)
                        )
                        part.tooltip = new vscode.MarkdownString('`lib.es2015.collection.ts`', true)
                        part.command = { command: 'helloWorld', title: 'Hello World', tooltip: 'dddd' }

                    } else if (part.value === 'typeof document') {
                        part.location = new vscode.Location(
                            vscode.Uri.file('/Users/jrieken/Code/vscode/extensions/node_modules/typescript/lib/lib.dom.d.ts'),
                            new vscode.Position(17160, 13)
                        )
                        part.tooltip = new vscode.MarkdownString('`lib.dom.ts`', true)
                    } else if (part.value === 'foo') {
                        part.command = {
                            command: 'helloWorld',
                            title: 'Hello World',
                            tooltip: 'dddd'
                        }
                        part.tooltip = new vscode.MarkdownString('I am a _label_ *part* $(check)', true)
                        part.location = new vscode.Location(
                            vscode.Uri.file('/Users/jrieken/Code/vscode/extensions/node_modules/typescript/lib/lib.es2015.collection.d.ts'),
                            new vscode.Position(20, 10)
                        )
                    }
                }
            }

            // console.log('DONE', hint.label)
            return hint;
        }
    })

@lnicola

This comment was marked as resolved.

@lnicola

This comment was marked as resolved.

@jrieken
Copy link
Member

jrieken commented Feb 10, 2022

Oh... That is very interesting and very good drilling 👏 It is likely that position isn't an instance of vscode.Position but just a lookalike. TypeScript doesn't enforce instanceof-compatibility, e.g its structured typing ignore prototypes or ctor-functions and therefore we don't successfully exercise contains. Today, you can workaround this by using new vscode.Position(...) but I will also push a fix to relax this

mustard-mh pushed a commit to gitpod-io/openvscode-server that referenced this issue Feb 13, 2022
mustard-mh pushed a commit to gitpod-io/openvscode-server that referenced this issue Feb 13, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 7, 2022
@Tyriar
Copy link
Member Author

Tyriar commented Aug 9, 2022

This isn't working as I was expecting it to, ctrl+click goes to the symbol the type is annotating, not the type itself.

Recording 2022-08-09 at 06 03 41

Cursor feedback is also not great

@mjbvz
Copy link
Collaborator

mjbvz commented Aug 9, 2022

@Tyriar I think that's the default behavior (if you look closely, I believe ctrl+hover actually underlines the var in this case)

We haven't actually hooked this up for JS/TS yet but I'll open a TS issue for this

microsoft/TypeScript#50236

@Tyriar
Copy link
Member Author

Tyriar commented Aug 9, 2022

@mjbvz makes sense, thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api feature-request Request for new features or functionality inlay-hints on-release-notes Issue/pull request mentioned in release notes on-testplan
Projects
None yet
Development

No branches or pull requests

9 participants