Properly handle disabling of the LSP package #2085
Conversation
* main: Filter out non-quickfix actions in view (#2081) Fix crash on generating schema with mixed-type enum Cut 1.19.0
* origin/main: Only enable Goto Diagnostic commands if diagnostic with configured severity exists (#2091)
* origin/main: Add `$line` and `$character` expansion variables (#2092)
* origin/main: fix issues with restarting servers (#2088)
|
For now I'm done. I did some refactorings that should make it behave better when package is disabled/unloaded. Don't think everything is perfect yet but it should be getting there. I realize the changes are hard to review so one might rely on tests more here. :) |
|
Actually I've just tested disabling the LSP package and it didn't really address the issue with panels not being destroyed. Or, they are destroyed but due to async nature they come back immediately when remaining requests are being handled. Since now all panels are controlled through the same path (PanelManager) it's easier to prevent that but I'm a bit hesitant to add many repetitive checks... EDIT: addressed in later commit |
|
Think nobody wants to waste their life reviewing it properly so life will review it. |
pretty much like the code changes in this PR but haven't tested it :) |
* main: Focus symbol closest to selection on showing document symbols Properly handle disabling of the LSP package (#2085) fix issues with restarting servers (#2088) Add `$line` and `$character` expansion variables (#2092) Only enable Goto Diagnostic commands if diagnostic with configured severity exists (#2091) Update diagnostics gutter icons and change default to "sign" (#2086) Fix short color box rendering bug after color presentation change (#2087)
Trying to better handle cleanup on disabling the LSP package and kill some leaks:
WindowRegistry.lookupreturn value fromWindowManagertoOptional[WindowManager]. This ensures that when the LSP package gets disabled, existing references doing a lookup won't be able to create new instances ofWindowManager.panel.pygo throughWindowManagerwhen wanting to create output panel forWindowso that they don't create panels when package is disabled.globalprefs) onplugin_unloadedas that can cause crashes in code that is still runninguserprefonplugin_unloadedso that accessing settings doesn't crashConfigManagerand makeWindowRegistryresponsible for creatingWindowConfigManagers and passing it toWindowManager.configsand (?)globals. Maybe those should live and be controlled throughclient_configsWindowRegistryso that we have least amount of globals possible.Note that due to ST bug sublimehq/sublime_text#5148, we don't currently remove and delete instances ofThe bug was fixed.WindowManagerso added a workaround for that so that we remove no longer valid instances now and then.Should fix #2082 when completed.