search

5hirish / quinine

A Flutter IDE by developers for developers (In Development)
https://discord.gg/Yh5Nkrznaw
Apache License 2.0
107 stars 8 forks source link

LSP Integration: M1 - Server <> client initialise handshake #17

Closed 5hirish closed 1 year ago

5hirish commented 1 year ago

LSP Integration: Milestone 1

Description

Integrates Dart LSP server offered with Dart-SDK. Quinine creates a Dart LSP process and exposes it as provider. An abstract LSP class implements all client methods that are supported by the Dart LSP server. On user workspace changes via UI (selecting a new workspace or changed to an existing workspace), process is started if not, and the first initialise request is made to the dart server to confirm and register capabilities and initialise it. After which server notifies as initialised. If already started and initialised the server simply changes workspace.

Fixes #9 Progress made

Type of change

Please delete options that are not relevant.

Checklist:

codecov-commenter commented 1 year ago

Codecov Report

Merging #17 (2dbfffc) into master (4d7ede5) will increase coverage by 11.02%. The diff coverage is 57.52%.

@@             Coverage Diff             @@
##           master      #17       +/-   ##
===========================================
+ Coverage   28.52%   39.55%   +11.02%     
===========================================
  Files          34       54       +20     
  Lines         666     1067      +401     
===========================================
+ Hits          190      422      +232     
- Misses        476      645      +169
Files Changed Coverage Δ
lib/models/lsp/error.dart 0.00% <0.00%> (ø)
lib/models/lsp/params/clientInfo.dart 0.00% <0.00%> (ø)
lib/models/lsp/params/document/range.dart 0.00% <0.00%> (ø)
...dels/lsp/params/document/textDocContentChange.dart 0.00% <0.00%> (ø)
lib/models/lsp/params/document/textDocItem.dart 0.00% <0.00%> (ø)
...models/lsp/params/document/versionedTextDocId.dart 0.00% <0.00%> (ø)
lib/models/lsp/params/initializationOptions.dart 0.00% <0.00%> (ø)
lib/models/lsp/params/initialize.dart 0.00% <0.00%> (ø)
lib/models/lsp/params/workspaceFolder.dart 0.00% <0.00%> (ø)
lib/models/notification.dart 0.00% <0.00%> (ø)
... and 17 more
5hirish commented 1 year ago

In many scenarios, sending the shutdown to the LSP server when the app goes into the background might be acceptable since you'll likely want to start it fresh when resuming the app anyway. But this depends on the specifics of your app's functionality and how costly starting the LSP server is.

To better handle the LSP lifecycle

  1. When paused is triggered, you can start a timer.
  2. If the resumed state is triggered before the timer finishes, then you know the app was just in the background momentarily and has now come to the foreground. You can then cancel the timer.
  3. If the timer completes (i.e., after a certain duration without the resumed state being triggered), you assume the app won't be coming back shortly and send the shutdown request to the LSP server.
5hirish commented 1 year ago

Flutter Desktop currently does not support for life cycle events. https://github.com/flutter/flutter/issues/103637

5hirish commented 1 year ago

Might require revisiting the dart LSP providers implemented here after flutter introduces multi-window desktop support.

IDE supporting multi-window (or multi-instance or multi-projects) functionality, each window/project should ideally have its own instance of the LSP server. This ensures that each project is isolated in terms of linting, autocompletion, etc.

Also, it's essential to maintain a clean lifecycle for each LSP server instance. Always ensure you send the shutdown and exit commands before closing an instance to free up resources and ensure there are no lingering processes.

Track flutter desktop multi-window support here - https://github.com/flutter/flutter/issues/103637

5hirish commented 1 year ago

Although the dart LSP provider is keepAlive: true. When you have a provider with keepAlive: true, it means that the provider will be kept alive as long as there's at least one listener. A keepAlive provider won't be immediately destroyed when there are no more listeners, giving an opportunity to be reused if another part of the app starts listening to it again.

However, keepAlive doesn't mean that the provider will never be destroyed. It can be destroyed in these scenarios:

  1. Application Closure: When you completely close the application, all resources, including providers, will be disposed of.

  2. Parent Provider Re-creation: If your dartLSPProvider depends on another provider and that parent provider is recreated (its state changes in a way that it rebuilds), the child (in this case, the dartLSPProvider) might also get recreated, which will cause its previous instance to be disposed of.

  3. Explicitly Using Refresh: If you use the refresh method on the provider, it will be re-initialized, and the previous instance will be disposed of.

When the provider is disposed of, the onDispose callback you provided will be executed. So, if you have your LSP shutdown logic in onDispose, it will run in the above scenarios.

Regarding the closure of the app, the cleanup will happen as part of the Flutter framework's standard lifecycle. However, as discussed earlier, it's a good idea to handle shutdown signals on desktop platforms explicitly to ensure all resources are released properly. This is particularly important for resources like processes, network connections, etc., to avoid any unintentional lingering processes.