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

Extend language API to allow sourceRange in Go to Definition #10037

Closed
vidhya03 opened this issue Aug 2, 2016 · 15 comments · Fixed by #52230
Closed

Extend language API to allow sourceRange in Go to Definition #10037

vidhya03 opened this issue Aug 2, 2016 · 15 comments · Fixed by #52230
Assignees
Labels
api-proposal feature-request Request for new features or functionality on-testplan plan-item VS Code - planned item for upcoming typescript Typescript support issues
Milestone

Comments

@vidhya03
Copy link

vidhya03 commented Aug 2, 2016

  • VSCode Version: Code 1.3.1 (e6b4afa, 2016-07-12T13:35:06.227Z)
  • OS Version: Windows_NT ia32 6.1.7601

Steps to Reproduce:

  1. Create a typescript project for eg angular2
  2. create a component with name having dot character
  3. import the module in to another component.

ctrl + hover - highlighting upto dot character
in the below issue the ctl + hover should highlight gateway.destination.component but it is highlighting only the component alone.
vscode-ctrl-hover

@dbaeumer dbaeumer assigned alexdima and unassigned dbaeumer Aug 3, 2016
@dbaeumer
Copy link
Member

dbaeumer commented Aug 3, 2016

This is the link detector / renderer in the editor. The tsserver returns the right result for the definition request. @alexandrudima assuming this is you. Please move back if I am mistaken.

@alexdima
Copy link
Member

I think it is a limitation of DefinitionProvider that does not return the source range, it only returns the definition ranges.

@alexdima alexdima added the feature-request Request for new features or functionality label Aug 12, 2016
@alexdima
Copy link
Member

fyi @bpasero @jrieken

@alexdima alexdima removed their assignment Aug 12, 2016
@bpasero
Copy link
Member

bpasero commented Aug 12, 2016

Yes, I am getting the target location and so the link is being created around the word you hover over. We have the same issue in selfhost when hovering over imports too.

See https://github.com/Microsoft/vscode/blob/master/src/vs/editor/contrib/goToDeclaration/browser/goToDeclaration.ts#L276

@jrieken
Copy link
Member

jrieken commented Aug 12, 2016

@bpasero You could maybe not use the word but the token? Then it would be the whole string token instead of the string in there?

@bpasero
Copy link
Member

bpasero commented Aug 12, 2016

@jrieken is there API for me to get this info? I can give it a try 👍

@jrieken
Copy link
Member

jrieken commented Aug 12, 2016

Yeah - I believe you use getLineToken-method and continue from there

@bpasero
Copy link
Member

bpasero commented Aug 12, 2016

I cannot use tokens because some languages return a lot more in a single token compared to what you can click on. For example for a TS thing like QUnit.equal(entry, entry.next); the token includes QUnit, equal and even the whitespace to the left.

I think the lack of token info here was the original reason why we went with words.

@alexdima
Copy link
Member

I think this would go well only if the GoToDefinition result would include a sourceRange or something that could be used for highlighting. Relying on the tokens has the exact same kind of issues as relying on the word.

@dbaeumer dbaeumer added the typescript Typescript support issues label Aug 15, 2016
@dbaeumer
Copy link
Member

@jrieken @ben: who should own this. To my knowledge the API doesn't allow to return the information and the tsserver doesn't provide it.

@jrieken
Copy link
Member

jrieken commented Aug 15, 2016

No language service I know off provides that information. I still believe that tokens are the way to go but 'yes' it does depend on how good/bad those are. If we extend the API to allow for a source range we still need to handle the missing case.

@bpasero
Copy link
Member

bpasero commented Aug 15, 2016

As of today using tokens is worse than using words :)

@waderyan waderyan changed the title TypeScript ctrl + hover highlighting incomplete Extend language API to allow sourceRange in Go to Definition Nov 21, 2016
@mjbvz mjbvz modified the milestone: Backlog Feb 23, 2017
@waderyan waderyan removed their assignment Jul 13, 2017
@mjbvz
Copy link
Collaborator

mjbvz commented Oct 31, 2017

Upstream change from TS to support this: microsoft/TypeScript#19175

@mjbvz mjbvz modified the milestones: Backlog, November 2017 Oct 31, 2017
@mjbvz mjbvz modified the milestones: February 2018, March 2018 Feb 27, 2018
mjbvz added a commit that referenced this issue Mar 8, 2018
Supersedes #40045

**Problem**
See #10037

**Proposal**
Adds a new `resolveDefiniotionContext` to `DefinitionProvider`. This allows a definition provider to return the defining symbol span.
@mjbvz mjbvz added the plan-item VS Code - planned item for upcoming label Mar 16, 2018
@mjbvz mjbvz modified the milestones: March 2018, April 2018 Mar 23, 2018
mjbvz added a commit to mjbvz/vscode that referenced this issue Apr 16, 2018
**Problem**
See microsoft#10037

**Proposal**
Add a new `SymbolDefinition` class to the VSCode API. This bundles up a location and the span of the defining symbol
@mjbvz mjbvz modified the milestones: April 2018, May 2018 Apr 23, 2018
@mjbvz mjbvz modified the milestones: May 2018, June 2018 May 24, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-proposal feature-request Request for new features or functionality on-testplan plan-item VS Code - planned item for upcoming typescript Typescript support issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants