SymbolDefinition API #48001
Closed
SymbolDefinition API #48001
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**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
b15156a to
bfa410f
Compare
jrieken
requested changes
Apr 17, 2018
| // get results | ||
| const promises = provider.map((provider, idx): TPromise<Location | Location[]> => { | ||
| return asWinJsPromise((token) => { | ||
| return provide(provider, model, position, token); | ||
| }).then(undefined, err => { | ||
| }).then(result => { | ||
| if (result && (result as any).definitions) { |
| //#region Defintion symbol range: mjbvz | ||
|
|
||
| export interface SymbolDefinition { | ||
| definingSpan?: Location; |
| export interface SymbolDefinition { | ||
| definingSpan?: Location; | ||
|
|
||
| definitions: Location[]; |
There was a problem hiding this comment.
Needs room for a target range, e.g. the full range of the target vs the navigate-to-target. That's needed for the source preview hover (can be delayed but must happen before making this 'real' api)
There was a problem hiding this comment.
An alternative could also be to let SymbolDefinition talk about just one link and use it as array. So instead of definitions: Location[]; return SymbolDefinition[]. I assume that would help with additional metadata for a single link, e.g. the actual symbol ranges etc
| @@ -520,6 +520,10 @@ export interface Location { | |||
| */ | |||
| range: IRange; | |||
| } | |||
| export interface SymbolDefinition { | |||
| definingSpan?: Location; | |||
There was a problem hiding this comment.
Making this optional is like return Definitions... I'd be in favour of strict, return either a 'normal' definition or a rich definition
| data.definingSpan = this._reviveLocationDto(data.definingSpan); | ||
| } | ||
|
|
||
| data.definitions = data.definitions.map(x => this._reviveLocationDto(x)); |
There was a problem hiding this comment.
No need to map or assign because revive changes objects in-place (for the sake of avoiding garbage). Same on line 80
| return wireCancellationToken(token, this._proxy.$provideDefinition(handle, model.uri, position)).then(MainThreadLanguageFeatures._reviveLocationDto); | ||
| provideDefinition: (model, position, token): Thenable<modes.Definition | modes.SymbolDefinition> => { | ||
| return wireCancellationToken(token, this._proxy.$provideDefinition(handle, model.uri, position)).then(x => { | ||
| if ((x as any).definitions) { |
|
|
||
| if (Array.isArray(value)) { | ||
| return value.map(TypeConverters.location.from); | ||
| } else if ((value as any).definitions) { |
| provide: (provider: T, model: ITextModel, position: Position, token: CancellationToken) => Location | Location[] | Thenable<Location | Location[]> | ||
| ): TPromise<Location[]> { | ||
| provide: (provider: T, model: ITextModel, position: Position, token: CancellationToken) => Location | Location[] | SymbolDefinition | Thenable<Location | Location[] | SymbolDefinition> | ||
| ): TPromise<SymbolDefinition> { |
There was a problem hiding this comment.
This changes how the corresponding API-command works. We should update the doc around that and inform live share folks
|
|
||
| //#region Defintion symbol range: mjbvz | ||
|
|
||
| export interface SymbolDefinition { |
Merged
|
Replaced by #52230 |
mjbvz
added a commit
that referenced
this pull request
Jun 19, 2018
Add a new `DefinitionLink` type. This type allows definition providers to return additional metadata about a definition, such as the defining span. Hook up this new provider for typescript This PR replaces #48001
mjbvz
added a commit
to mjbvz/vscode
that referenced
this pull request
Jun 20, 2018
Add a new `DefinitionLink` type. This type allows definition providers to return additional metadata about a definition, such as the defining span. Hook up this new provider for typescript This PR replaces microsoft#48001
mjbvz
added a commit
that referenced
this pull request
Jun 20, 2018
* Definition link Add a new `DefinitionLink` type. This type allows definition providers to return additional metadata about a definition, such as the defining span. Hook up this new provider for typescript This PR replaces #48001 * Correctly mark field optional * Small code fixes - Use lift - Remove unused param * Adding documentation
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
#10037 again.
Proposal
Add a new
SymbolDefinitiontype to the VSCode API. This bundles up set of definitions and the span of the defining symbolThis PR supersedes #45249 and returns to the approach from #40045