Conversation
**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
| 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
| 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 { |
|
Replaced by #52230 |
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
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
* 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
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