This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Publish config file errors over LSP
ClosedPublic

Authored by sammccall on Dec 4 2020, 3:40 PM.

Details

Summary

We don't act as a language server for these files (e.g. don't get open/close
notifications for them), but just blindly publish diagnostics for them.

This works reasonably well in coc.nvim and vscode: they show up in the
workspace diagnostic list and when you open the file.
The only update after the file is reparsed, not as you type which is a bit
janky, but seems a lot better than nothing.

Fixes https://github.com/clangd/clangd/issues/614

Diff Detail

Event Timeline

sammccall created this revision.Dec 4 2020, 3:40 PM
sammccall requested review of this revision.Dec 4 2020, 3:40 PM
sammccall edited the summary of this revision. (Show Details)Dec 4 2020, 3:45 PM
sammccall updated this revision to Diff 309688.Dec 4 2020, 3:54 PM

Handle diagnostics without a filename

hokein accepted this revision.Dec 7 2020, 1:05 AM

The pre-merging bot seems to be failed on windows: https://reviews.llvm.org/harbormaster/unit/view/215879/.

clang-tools-extra/clangd/ClangdServer.cpp
833

I'd encode Config into names, DiagnosticHandler made me think this is a handler for clang diagnostics (without reading the context).

clang-tools-extra/clangd/ClangdServer.h
366

nit: = nullptr.

This revision is now accepted and ready to land.Dec 7 2020, 1:05 AM
This revision was landed with ongoing or failed builds.Dec 7 2020, 2:07 AM
This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
thakis added a subscriber: thakis.Dec 7 2020, 3:22 AM

Looks like this breaks tests on windows: http://45.33.8.238/win/29315/step_9.txt

Yep, thanks. This uncovered an existing error, so rather than reverting the
whole thing I've disabled the assert on windows while I fix
it. f1357264b8e3070bef5bb4ff35ececa4d6c76108