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

Forces refresh of a FileObject after the LSP client reports the file has been saved. #7729

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Sep 4, 2024

If the LSP client makes file changes, and then immediately performs some NBLS operation, the NBLS currently does not get a timely file change event. This PR enables didSave notification that forces a FileObject refresh, firing appropriate events.
Somewhat controversial is the wait for reloading task, so the affected editor is reloaded before a next instruction (i.e. text modification) is received from the LSP commnand stream.

@sdedic sdedic added LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests labels Sep 4, 2024
@sdedic sdedic added this to the NB24 milestone Sep 4, 2024
@sdedic sdedic self-assigned this Sep 4, 2024
@sdedic sdedic requested review from dbalek and jhorvath September 10, 2024 11:58
@sdedic sdedic marked this pull request as ready for review September 10, 2024 11:58
@sdedic sdedic requested a review from lahodaj September 13, 2024 07:33
Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like the best we can do now. I assume if the document is modified, the modifications are thrown away on the reload automatically, correct?

@sdedic
Copy link
Member Author

sdedic commented Sep 13, 2024

This seems like the best we can do now. I assume if the document is modified, the modifications are thrown away on the reload automatically, correct?

Yes, the answer to on-reload question is API-branded to "yes" in the NBLS distribution.

Copy link
Contributor

@jhorvath jhorvath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@lahodaj
Copy link
Contributor

lahodaj commented Sep 13, 2024

This seems like the best we can do now. I assume if the document is modified, the modifications are thrown away on the reload automatically, correct?

Yes, the answer to on-reload question is API-branded to "yes" in the NBLS distribution.

Great, thanks!

@sdedic sdedic merged commit 51dea0c into apache:master Sep 13, 2024
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants