lsp4j icon indicating copy to clipboard operation
lsp4j copied to clipboard

change Launcher startListening result to CompletableFuture?

Open frankbenoit opened this issue 2 years ago • 1 comments

Hi Lsp4J,

I wonder if it would make sense to have a CompletableFuture as the result type of the JSON-RPC Launcher#startListening? The Future is create here: org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.startProcessing(MessageProducer, MessageConsumer, ExecutorService) org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.beginProcessing(ExecutorService) Could it be changed to CompletableFuture.runAsync( reader/this, executorService )

In the ChatExample from here: https://www.eclipse.org/community/eclipse_newsletter/2017/may/article2.php The class SocketLauncher is mainly there to do the conversion Future -> CompletableFuture and creates a blocking thread to do so. This seems waste to me.

Frank

frankbenoit avatar Nov 28 '21 08:11 frankbenoit

Hi Frank,

Thank you for your suggestion, it sounds interesting.

Until someone with a deeper knowledge of the subject replies, here are my two cents:

  1. Obviously, this would be a breaking API change. In particular, it could break any existing implementation of the Launcher interface.

  2. There is a difference in behavior of the cancel method of a Future returned from ExecutorService.submit and a CompletableFuture returned from CompletableFuture.runAsyc. The latter ignores the mayInterruptIfRunning parameter, i.e. it will not try to stop the running task.

pisv avatar Nov 28 '21 11:11 pisv