-
Notifications
You must be signed in to change notification settings - Fork 321
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
Fixes for Language Server sync server #8514
Conversation
@@ -386,9 +386,7 @@ export namespace response { | |||
contentRoots: ContentRoot[] | |||
} | |||
|
|||
export interface FileContents { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the FileReadResult
type, but improperly named. According to docs it does have a nested contents
field. So it was probably allright, bar the incorrect name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually i discovered this was incorrect, when i was implementing this:
enso/app/gui2/ydoc-server/languageServerSession.ts
Lines 585 to 596 in 2d2c9c9
case LsSyncState.Synchronized: { | |
this.withState(LsSyncState.Reloading, async () => { | |
const promise = Promise.all([ | |
this.ls.readFile(this.path), | |
this.ls.fileChecksum(this.path), | |
]) | |
this.setLastAction(promise) | |
const [contents, checksum] = await promise | |
this.syncFileContents(contents.contents, checksum.checksum) | |
}) | |
return | |
} |
on that note... i'm not 100% sure about this. it doesn't affect the fix if it's removed - it's entirely possible that I've misunderstood what reload
is supposed to do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that there is a FileContents
type: https://github.com/enso-org/enso/blob/develop/docs/language-server/protocol-language-server.md#filecontents
also note that *Result
types were basically only added so that the Result
interfaces are valid TS code - in GUI2 these should correspond to response.*
Pull Request Description
Important Notes
text/openFile
(openTextFile
) is still present, but (seemingly?) harder to repro nowChecklist
Please ensure that the following checklist has been satisfied before submitting the PR:
he documentation has been updated, if necessary.Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible../run ide build
.