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

Question about the order of LSP requests and notifications (didChange and semanticTokens) #1495

Closed
He1pa opened this issue Jun 11, 2024 · 6 comments

Comments

@He1pa
Copy link

He1pa commented Jun 11, 2024

I am a developer of the lsp language extension. I noticed that under normal circumstances, when a user enters code in vscode, lsp will:
1 Send a notification of file changes (Notification { method: "textDocument/didChange"})
2 Request new semantic highlighting (Request { method: "textDocument/semanticTokens})

But I found that when inputting continuously and quickly (such as typing two Enter or other keys), sometimes semantic highlighting will be requested first, and then file changes will be notified.

I'm not sure if this is a problem with my lsp client implementation, or a special design of vscode, or a configurable option of vscode

here is my language extension:
client: https://github.com/kcl-lang/vscode-kcl
server: https://github.com/kcl-lang/kcl/tree/main/kclvm/tools/src/LSP

also reported in microsoft/vscode-discussions#1219

Version: 1.89.1 (Universal)
Commit: dc96b837cf6bb4af9cd736aa3af08cf8279f7685
Date: 2024-05-07T05:14:24.611Z
Electron: 28.2.8
ElectronBuildId: 27744544
Chromium: 120.0.6099.291
Node.js: 18.18.2
V8: 12.0.267.19-electron.0
OS: Darwin arm64 21.5.0

@dbaeumer
Copy link
Member

This behavior is absolutely fine. The server should in this situation compute the semantic tokens on the state of files it knows. If then some additional typing happens on the client, the client should either

  • send a cancel event and re-issue a new semantic token request after the did change has been sent.
  • keep the request running and adjust the result on the client side if possible. Otherwise issue a new request.

Hope that explains it,

@He1pa
Copy link
Author

He1pa commented Jun 12, 2024

I don't fully understand. Let me describe the problem I encountered in more detail.
At first, my code was like this.

schema Data:
    name: str
    version?: str

I typed Enter twice at the beginning of the file,



schema Data:
    name: str
    version?: str

I got the log of lsp server

Notification(Notification { method: "textDocument/didChange", params: Object {"contentChanges": Array [Object {"text": String("\nschema Data:\n    name: str\n    version?: str\n")}], "textDocument": Object {"uri": String("file:///Users/zz/code/test/a.k"), "version": Number(35)}} })

Notification(Notification { method: "textDocument/didChange", params: Object {"contentChanges": Array [Object {"text": String("\n\nschema Data:\n    name: str\n    version?: str\n")}], "textDocument": Object {"uri": String("file:///Users/zz/code/test/a.k"), "version": Number(36)}} })

Request(Request { id: RequestId(I32(132)), method: "textDocument/semanticTokens/full", params: Object {"textDocument": Object {"uri": String("file:///Users/zz/code/test/a.k")}} })

For each didChanged notification, a \n is added to the text

But if I typed delete twice

schema Data:
    name: str
    version?: str

I got the log of lsp server

Request(Request { id: RequestId(I32(138)), method: "textDocument/semanticTokens/full", params: Object {"textDocument": Object {"uri": String("file:///Users/zz/code/test/a.k")}} })

Notification(Notification { method: "textDocument/didChange", params: Object {"contentChanges": Array [Object {"text": String("schema Data:\n    name: str\n    version?: str\n")}], "textDocument": Object {"uri": String("file:///Users/zz/code/test/a.k"), "version": Number(38)}} })

The semanticTokens request occurs before didChange, and the didChange notification is only the second time

@dbaeumer
Copy link
Member

Ah, now I understand. This got fix in the latest versions of the client (e.g. 9.x)

@He1pa
Copy link
Author

He1pa commented Aug 6, 2024

It seems fixed by #1511.
From my observation, it seems that the two consecutive operations will be merged into the final state. And the order of requests is normal.
For the previous case:

Notification(Notification { method: "textDocument/didChange", params: Object {"contentChanges": Array [Object {"text": String("schema Data:\n    name: str\n    version?: str\n")}], "textDocument": Object {"uri": String("file:///Users/zz/code/test/a.k"), "version": Number(38)}} })
Request(Request { id: RequestId(I32(138)), method: "textDocument/semanticTokens/full", params: Object {"textDocument": Object {"uri": String("file:///Users/zz/code/test/a.k")}} })

But I still have a small question, can I check the version of vscode-languageserver-node I am using in the VSCode to ensure that?

@dbaeumer
Copy link
Member

dbaeumer commented Aug 6, 2024

You define the version you are using via the npm dependency (vscode-languageclient)

@He1pa
Copy link
Author

He1pa commented Aug 6, 2024

You define the version you are using via the npm dependency (vscode-languageclient)

thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants