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

Fixes for DialogNotifier in LSP #4907

Merged
merged 3 commits into from
Nov 8, 2022

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Nov 3, 2022

The main issue fixed is the binding between LSP UIContext and the first client that initializes it and then is stuck with it. The original code gave the 1st client the priority over the client that may have been present in the computational context Lookup.getDefault(). The changed code will use the current Lookup.getDefault() that is transferred through various RP.Tasks - this is already implemented in UIContext.find()

  • fixes ADM report, so it uses fire-and-forge notifyLater instead notify. The LSP client may use modeless UI (vscode does) and the server thread is left blocked until the user dismisses the report (in vscode the user may just ignore it or hide it into status line without dismissing)
  • minor fix for tests: the structure must gson-deserialize on JDK11, so default ctor + setters are needed

@sdedic sdedic added LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests labels Nov 3, 2022
@sdedic sdedic added this to the NB17 milestone Nov 3, 2022
@sdedic sdedic requested review from ppisl and dbalek November 3, 2022 10:33
@sdedic sdedic self-assigned this Nov 3, 2022
Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

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

Looks fine

@sdedic sdedic merged commit 907cf07 into apache:master Nov 8, 2022
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.

None yet

3 participants