Repository navigation
Fix restartLanguageServer command not found in command palette (#22045) - #25996
aryan maheshwari (arymaheshwari) wants to merge 4 commits into
Conversation
…soft#22045) - Move python.analysis.restartLanguageServer command registration from individual language server managers to LanguageServerWatcher.register(), so it is always available regardless of which language server type is active - Fix async/forEach bug in restartLanguageServers() — forEach ignores returned Promises, so awaits inside were silently dropped; replaced with for...of - Fix resource URI reconstruction in restartLanguageServers() — map keys 'Pylance'/'None' are not URIs, Jedi keys are fsPath strings needing Uri.file() not Uri.parse() - Remove now-redundant static commandDispose and command registration from NodeLanguageServerManager and JediLanguageServerManager - Update unit tests: fix disposable count assertions, fix ICommandManager mocks, add test for restart command handler
|
@microsoft-github-policy-service agree |
|
Rich Chiodo (@rchiodo) Bill Schnurr (@bschnurr) Stella Huang (@StellaHuang95) could I get a review of this to confirm this is the desired behavior for this interaction |
|
The implementation appears to support Pylance through |
Eleanor Boyd (eleanorjboyd)
added
the
bug
|
Rich Chiodo (@rchiodo) what do you think about my above comment? |
Sounds good to me. Add a dummy pylance implementation and make sure it gets closed in the test? |
Fixes #22045
Problem
The
python.analysis.restartLanguageServercommand was previously owned by Pylance and was removed from it. The vscode-python extension registered the command handler inside individual manager constructors (NodeLanguageServerManager,JediLanguageServerManager), so it was only available when Pylance or Jedi was active — never whenLanguageServerType.Nonewas in use.Additionally,
LanguageServerWatcher.restartLanguageServers()had an async/forEach bug that silently discarded all awaits, and was never called from anywhere.Changes
watcher.ts: RegisterRestartLScommand inregister()so it is always active; fixrestartLanguageServers()withfor...ofand correct URI handlingnode/manager.ts: Remove staticcommandDisposeand command registration from constructorjedi/manager.ts: Same cleanupwatcher.unit.test.ts: Fix disposable count assertions andICommandManagermocks; test command registration with no language server and restart through the real Pylance manager/proxy using a dummy Pylance client. Verify stop completes before restart, the old manager is disposed, and the restarted client is stopped during cleanup.Testing
npx tsc -p ..Unit tests were run against compiled JavaScript using the repository's
out/test/unittests.jsbootstrap, Mocha's TDD interface, and the four relevant test files. The new Pylance test uses a dummy extension API client; it does not require an installed Pylance extension.