Infrastructure to display simple confirmations/questions in LSP client. #2493
Conversation
| } | ||
|
|
||
| @Override | ||
| public CompletableFuture<InitializeResult> initialize(InitializeParams init) { |
There was a problem hiding this comment.
Note this change: some events are fired synchronously from Open Project. If the code asks for something like DialogDisplayer.notify (which some code really does), the initialization request processing would block - and the notify() round-trip would be never completed, freezing LSP client-server communication completely.
| * | ||
| * @author sdedic | ||
| */ | ||
| //@ServiceProvider(service = DialogDisplayer.class, position = 1000) |
There was a problem hiding this comment.
Reasonable, given we after feature freeze. This functionality will certainly be needed in the future. For 12.2 we'd rather delay using it, unless necessary.
There was a problem hiding this comment.
I'm bit confused. The PR is against delivery, but no 12.2 milestone set, I'm not among the reviewers. Am I assuming right upon this comment, that you would like to float this here in case of need? So it is tentative for 12.2? If yes, please decide it till beta3 (29th Oct, late PDT) this week. If that one is messed this one have to go to master. I've put the milestone 12.2 anyway.
There was a problem hiding this comment.
Is beta3 supposed to become VC1? Oct 29, so close!
I asked Sváťa to propose his infrastructure changes in this PR for 12.2 as they are harmless (until we enable @ServiceProvider), but we may desperately need them. I am afraid we may need to download nbjavac if support for JDK8 is needed. Downloading shall of course only happen after asking a question - preferably through this Sváťa's infrastructure.
That's my rationale. but Oct 29, is so close!
There was a problem hiding this comment.
@lkishalmi I would added you after the PR passed through Jarda ;) it's easier to review cleaned-up code.
There was a problem hiding this comment.
The VC is or I'd better call it RC is due to 7th of November. Previously we had 2 week period between beta1 and beta2, I've switched it to one week as usually the forts week in the release mode is when the most commits are happening "oh, this is a minor, but needs to be there". Also the weekly delivery -> master merge is hopefully small enough change to deal with.
| } | ||
| } | ||
|
|
||
| private static final RequestProcessor SERVER_INIT_RP = new RequestProcessor(LanguageServerImpl.class.getName(), 10); |
There was a problem hiding this comment.
Throughput 10!? Isn't opening of project sequential anyway?
There was a problem hiding this comment.
Hmm ... OK; I thought about multiple instances that may have been initializing (multiple LSP clients), but you're right as far as there's just project open operation being waited on. Will change.
| } | ||
|
|
||
| private static final RequestProcessor SERVER_INIT_RP = new RequestProcessor(LanguageServerImpl.class.getName(), 10); | ||
| private static final Map<NbCodeClientWrapper, Future> initializations = new WeakHashMap<>(); |
There was a problem hiding this comment.
Is the initializations field read anywhere? I can't find it (and that'd be good, as I don't think it is needed).
There was a problem hiding this comment.
leftover, will remove. I originally planned to cancel the pending Future if the server receives exit()
| * @param original | ||
| * @return | ||
| */ | ||
| private String translateText(String original) { |
There was a problem hiding this comment.
I believe the translation of the text away from HTML shall only happen if it starts with <html>.
3 participants
I've implemented a
AbstractDialogDisplayerthat, if registered will remotenotify()andnotifyLaterthroughUIContextinto the remote LSP client. I've tested locally on Gradle projects; but it has still some limitations, like Project Problems tend to display a customizer ... so I've commented out the final registration of the service.I'd like to pull in the infrastructure for collaboration.