[PATCH] D44158: [clangd:vscode] Resolve symlinks for file paths from clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

LG. Can you expand the comment and fix the formatting changes?



================
Comment at: clangd/clients/clangd-vscode/src/extension.ts:3
 import * as vscodelc from 'vscode-languageclient';
+import { realpathSync } from 'fs';
 
----------------
nit: the braces don't do anything here, right?


================
Comment at: clangd/clients/clangd-vscode/src/extension.ts:28
     if (!!traceFile) {
-      const trace = {CLANGD_TRACE : traceFile};
-      clangd.options = {env : {...process.env, ...trace}};
+        const trace = { CLANGD_TRACE: traceFile };
+        clangd.options = { env: { ...process.env, ...trace } };
----------------
(this is an unrelated formatting change, so consider reverting anyway)
My clang-format doesn't add the whitespace inside {}, and the indentation change seems just wrong... what are you using to format?


================
Comment at: clangd/clients/clangd-vscode/src/extension.ts:41
+        },
+        // Resolve symlinks for all files provided by clangd.
+        // FIXME: it might make more sense to rely on clangd to handle symlinks.
----------------
I think this comment needs to be more precise, because it's important we know when to remove this.

This is a workaround for a bazel + clangd issue - bazel produces a symlink tree to build in, and  when navigating to the included file, clangd passes its path inside the symlink tree rather than its filesystem path. We work around this by resolving all symlinks on the client.
We should remove this once clangd knows enough about bazel to resolve the symlinks where needed (or if this causes problems for other workflows).

(that's not necessarily text that needs to go into the comment, but I think all the information does...)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44158