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

Support for Generators using LSP #1411

Open
H4ad opened this issue Jan 21, 2024 · 6 comments
Open

Support for Generators using LSP #1411

H4ad opened this issue Jan 21, 2024 · 6 comments
Assignees
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities
Milestone

Comments

@H4ad
Copy link

H4ad commented Jan 21, 2024

I have a PR opened to improve the json format extension for VS Code: microsoft/node-jsonc-parser#82 that uses generators to avoid creating a list twice at https://github.com/microsoft/vscode-json-languageservice/blob/5692bed6db771d0f8985f89a9ecd9e3bc96cbc00/src/utils/format.ts#L18-L20

I wonder if it is possible to support returning the Generator Function directly from the LSP, in this way, we can reduce the amount of memory needed to make edits in larger files.

In this case, the amount of memory needed to format the JSON file will be reduced to 471MB from 2.2GB (which is the amount of memory needed today).

I think this feature can be similar to stream interface/feature that vscode already have, but maybe we can expose that feature using Generators in this way, is much easier to developers to integrate.

@dbaeumer
Copy link
Member

I think this is possible on the LSP level itself. However since VS Code itself doesn't support any streaming the feature would be disable in LSP in VS Code is used as a client (as it is with streaming already today).

@dbaeumer dbaeumer transferred this issue from microsoft/vscode Jan 23, 2024
@dbaeumer dbaeumer added the feature-request Request for new features or functionality label Jan 23, 2024
@dbaeumer dbaeumer added this to the Backlog milestone Jan 23, 2024
@dbaeumer dbaeumer added the help wanted Issues identified as good community contribution opportunities label Jan 23, 2024
@H4ad
Copy link
Author

H4ad commented Jan 23, 2024

I have no knowledge of the architecture of LSP, do you have any files that I can read or links to understand how the TextEdit are being applied on vscode?

I was very confused about who consumes and applies the list of TextEdit returned to onFormattingDocument handlers.

If you think the support is simple, I can try to create a PR for that if you point to me which files are involved in this change.

@dbaeumer
Copy link
Member

The support is simple in LSP since it already has support for streaming (see https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#partialResults)

Add something like this to VS Code is hard since its API doesn't support streaming right now. See microsoft/vscode#105870 for the corresponding issue in VS Code.

@H4ad
Copy link
Author

H4ad commented Jan 30, 2024

Oh, I think now I understand, what you are saying is we basically can add support on the LSP of the language itself to PartialResults, and if the IDE supports that feature, it will use it.

In this case, vscode didn't support yet.

Until a few days ago, I knew nothing about LSP, I recently discovered that this is a protocol and it is being used by many IDEs, not only VSCode, so that's why I initially thought I could ask to vscode to add support.

Took me a few days to understand your phrase (and I hope I understand it now).

Anyway, let me know if I understand something incorrectly, I will study more about LSP and see how I can add support to partial results at https://github.com/microsoft/vscode-json-languageservice.

Feel free to close this issue, I think it will be more appropriate to open directly on vscode-json-languageservice, what do you think?

@H4ad
Copy link
Author

H4ad commented Jan 30, 2024

I probably understand something wrong, I found no example of usage of PartialResults, or no one ever tried to implement/add support for it, or I'm searching wrong, any thoughts?

@dbaeumer
Copy link
Member

There is one request kind that support partial results even in VS Code and that is the new pull diagnostics model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities
Projects
None yet
Development

No branches or pull requests

3 participants