review 3.15 spec#833
Conversation
|
@aeschli thanks. Ping me when ready to merge. |
|
@aeschli I can't find any todos in the PR :-). Any pointer? |
| * partial result: `DocumentSymbol[]` \| `SymbolInformation[]` | ||
| * error: code and message set in case an exception happens during the document symbol request. | ||
|
|
||
| TODO: can partial result be a mix of SymbolInformation and DocumentSymbol ? IMO we could limit it to SymbolInformation |
| * information about the error. Can be omitted. | ||
| */ | ||
| data?: D; | ||
| data?: string | number | boolean | number | array | object | null; |
There was a problem hiding this comment.
Sorry if I have misunderstood.
I think array is not included TypeScript primitive types.
So I think this is invalid type definition.
There was a problem hiding this comment.
Also, number is there twice.
There was a problem hiding this comment.
As the LSP is JSON, the valid types must be JSON data types. any is more than that.
Maybe we can define:
type LSPAny = LSPObject | LSPArray | string | number | boolean | null;
type LSPObject = { [key:string]:LSPAny };
type LSPArray = LSPAny[];and use it where needed.
| * The notification's params. | ||
| */ | ||
| params?: Array<any> | object; | ||
| params?: array | object; |
| * an error squiggle. | ||
| */ | ||
| export const Unnecessary: 1; | ||
| export const Unnecessary = 1; |
There was a problem hiding this comment.
This should be export const Unnecessary: 1 = 1;
| * Clients are allowed to rendered diagnostics with this tag strike through. | ||
| */ | ||
| export const Deprecated: 2; | ||
| export const Deprecated = 2; |
|
Are there any concrete plans on when this is targeted for release? Last release was also in December, so will we get a new version every December then? Or how are you deciding when to release? |
|
Merged the changes and addressed the open ToDos |
6 participants
Testing microsoft/vscode#81631
There are several TODO that are open questions (don't merge...)