fix: better handling when language server fails to start #454

dsherret · 2021-06-22T16:13:36Z

This is essentially #453, but without the parent-pid changes.

These changes also...

Allow users to update deno then just do the "Restart Language Service" command.
Allow users to update "deno.path" and have the changes take effect immediately.

… changes.

client/src/commands.ts

client/src/extension.ts

dsherret · 2021-06-22T16:17:12Z

client/src/extension.ts

+    // restart when "deno.path" changes
+    if (event.affectsConfiguration("deno.path")) {
+      vscode.commands.executeCommand("deno.restart");
+    }


Restarts the language server when someone changes "deno.path". This is a bit more convenient for users.

Side note: These command names and setting names should probably be moved to constants...

dsherret · 2021-06-22T16:28:50Z

client/src/util.ts

@@ -27,7 +25,7 @@ export async function getDenoCommand(): Promise<string> {
    // if sent a relative path, iterate over workspace folders to try and resolve.
    const list = [];
    for (const workspace of workspaceFolders) {
-      const dir = path.resolve(workspace.uri.path, command);
+      const dir = path.resolve(workspace.uri.fsPath, command);


Bug on windows. Should be fsPath in order to remove the leading slash (ex. shoudl be v:/folder and not /v:/folder).

client/src/types.d.ts

dsherret · 2021-06-22T16:31:31Z

client/src/util.ts

@@ -12,12 +15,7 @@ export function assert(cond: unknown, msg = "Assertion failed."): asserts cond {
  }
 }

-let memoizedCommand: string | undefined;


I don't think we should memoize this as it might prevent us from getting the latest information (ex. "deno.path" might have changed). This is only used when starting the language service anyway.

kitsonk

looks good, a couple of thoughts

client/src/commands.ts

client/src/extension.ts

client/src/types.d.ts

* Fix bug in `reloadImportRegistries` * Move SERVER_SEMVER to constants.ts

kitsonk

LGTM

* feat: code lens for references (denoland#308) * feat: disable most of builtin language service when deno enabled (denoland#307) * 0.0.8 * feat: add applyCodeActionCommand command (denoland#312) * fix: remove deno/applyCodeActionCommand (denoland#315) * New high-res logo (denoland#274) * feat: add implementations code lens configuration option (denoland#319) * feat: add initialize workspace command (denoland#316) * feat: support deno cache quick fix (denoland#322) * chore: add screenshot to README (denoland#323) * 0.0.9 * 0.0.10 * fix: typo in init command (denoland#327) * feat: add a welcome screen for extension (denoland#329) * feat: use preview instead of display for status (denoland#330) * chore: README improvements (denoland#331) * Release 3.0.0, canary is now main (denoland#332) * 3.0.1 * Fix typo (denoland#337) Grammar + context * chore: activate extension on command (denoland#336) * docs: recommend import_map.json instead of import-map.json (denoland#340) Resolves denoland#338 * chore: move Releases.md to CHANGELOG.md for better marketplace integration (denoland#344) Closes denoland#342 * feat: add deno.path setting (denoland#350) * 3.1.0 * feat: read-add debug support (denoland#351) Co-authored-by: CGQAQ <m.jason.liu@outlook.com> * feat: add settings to affect completions (denoland#368) * fix: manual `deno` command resolution on windows. (denoland#367) Fixes denoland#361 * 3.2.0 * feat: support for relative path resolution (using workspaces) in deno.path (denoland#381) Co-authored-by: Kitson Kelly <me@kitsonkelly.com> * feat: add version notification message (denoland#383) * feat: add restart language server command (denoland#385) Resolves denoland#372 * feat: add support for import registry completions (denoland#380) * 3.3.0 * typo in ImportCompletions.md (denoland#390) * fix: activate on reloadImportRegistries command (denoland#407) Fixes: denoland#394 * feat: handle per resource configuration (denoland#411) Requires Deno with denoland/deno#10488. Ref: denoland#348 Resolves: denoland#314 Resolves: denoland#297 * feat: add internalDebug config flag (denoland#406) Also integrate upstream formatting changes in deno fmt. Ref: denoland/deno#10368 * 3.4.0 * feat: recognise json(c) & markdown files (denoland#404) * 3.5.0 * docs: fix broken link in README (denoland#426) * feat: support registry auto discovery (denoland#427) * fix: bump semver of extension (denoland#429) * 3.5.1 * feat: add support for tasks and test code lens (denoland#436) * 3.6.0 * fix: update packaging and pin vsce version (denoland#440) Fixes denoland#439 * 3.6.1 * feat: add support for import map in test code lens (denoland#446) * fix: activate extension on markdown / json / jsonc (denoland#447) * fix: setting then clearing "deno.path" config should not use empty string for path (denoland#452) * fix: better handling when language server fails to start (denoland#454) * 3.7.0 * fix: remove trailing slash in example (denoland#471) * chore: remove test header from bug report issue template (denoland#479) * feat: add ability to set cache directory in settings (denoland#477) Closes denoland#287 * fix: properly handle plugin configuration at startup (denoland#474) Fixes denoland#473 Co-authored-by: Kitson Kelly <me@kitsonkelly.com> Co-authored-by: Luca Casonato <lucacasonato@yahoo.com> Co-authored-by: Kirill Reunov <Kirlovon@outlook.com> Co-authored-by: Liam Murphy <43807659+Liamolucko@users.noreply.github.com> Co-authored-by: Ryan Dahl <ry@tinyclouds.org> Co-authored-by: Jesse Jackson <jsejcksn@users.noreply.github.com> Co-authored-by: CGQAQ <m.jason.liu@outlook.com> Co-authored-by: David Sherret <dsherret@users.noreply.github.com> Co-authored-by: Hector Menendez <hector@gik.mx> Co-authored-by: Heyward Fann <fannheyward@users.noreply.github.com> Co-authored-by: Satya Rohith <me@satyarohith.com> Co-authored-by: yaegassy <yaegassy@users.noreply.github.com> Co-authored-by: Cedric Vangout <mail@cvng.dev>

dsherret added 9 commits June 18, 2021 12:37

Add support for the --parent-pid flag.

93bdfa2

Require deno v1.11.2 and use new --parent-pid flag

d2eba69

Actually handle --parent-pid flag and old version of Deno.

8e21194

Merge branch 'main' into lsp-add-parent-pid-flag

173d3ba

Start the language server last.

e7bea32

Add todo in execCommand

bdc4fe4

Revert the parent-pid changes in order to land a refactoring pr instead.

98c1e41

Only restart language server on configuration change when "deno.path"…

7eb0f19

… changes.

Burry complexity in ts_api.ts.

dd06afe

dsherret commented Jun 22, 2021

View reviewed changes

dsherret marked this pull request as ready for review June 22, 2021 16:31

dsherret requested a review from kitsonk June 22, 2021 18:07

kitsonk reviewed Jun 22, 2021

View reviewed changes

client/src/commands.ts Outdated Show resolved Hide resolved

client/src/commands.ts Outdated Show resolved Hide resolved

client/src/extension.ts Show resolved Hide resolved

client/src/types.d.ts Show resolved Hide resolved

Address feedback:

52aef02

* Fix bug in `reloadImportRegistries` * Move SERVER_SEMVER to constants.ts

dsherret changed the title ~~fix: better handling of language client being undefined~~ fix: better handling when language server fails to start Jun 23, 2021

kitsonk approved these changes Jun 23, 2021

View reviewed changes

dsherret merged commit 6bf0454 into denoland:main Jun 24, 2021

dsherret deleted the improve-language-client-undefined branch June 24, 2021 20:25

LumaKernel mentioned this pull request Aug 7, 2021

feat: merge v3.7.0 LumaKernel/coc-denoland#8

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: better handling when language server fails to start #454

fix: better handling when language server fails to start #454

dsherret commented Jun 22, 2021 •

edited

Loading

dsherret Jun 22, 2021

dsherret Jun 22, 2021

dsherret Jun 22, 2021

kitsonk left a comment

kitsonk left a comment

fix: better handling when language server fails to start #454

fix: better handling when language server fails to start #454

Conversation

dsherret commented Jun 22, 2021 • edited Loading

dsherret Jun 22, 2021

Choose a reason for hiding this comment

dsherret Jun 22, 2021

Choose a reason for hiding this comment

dsherret Jun 22, 2021

Choose a reason for hiding this comment

kitsonk left a comment

Choose a reason for hiding this comment

kitsonk left a comment

Choose a reason for hiding this comment

dsherret commented Jun 22, 2021 •

edited

Loading