This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Return empty results on spurious completion triggers
ClosedPublic

Authored by ilya-biryukov on Jun 7 2019, 1:46 AM.

Details

Summary

We currently return an error, this causes coc.nvim and VSCode to
show an error message in the logs.

Returning empty list of completions allows to avod the error message
without altering other user-visible behavior.

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.Jun 7 2019, 1:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2019, 1:46 AM
sammccall accepted this revision.Jun 7 2019, 2:13 AM
sammccall added inline comments.
clang-tools-extra/clangd/ClangdLSPServer.cpp
734 ↗(On Diff #203514)

this error code seems bogus, we should have a comment explaining that returning and empty list breaks vscode and any other error code breaks coc.nvim.

(Might want to file a vscode or LSP bug - trigger char is a heuristic, if [] isn't the preferred way for a server to say "no" then we should spell it out, if it is then vscode is wrong)

This revision is now accepted and ready to land.Jun 7 2019, 2:13 AM
ilya-biryukov retitled this revision from [clangd] Return 'RequestCancelled' on spurious completion triggers to [clangd] Return empty results on spurious completion triggers.Jun 7 2019, 9:06 AM
ilya-biryukov edited the summary of this revision. (Show Details)
  • Return empty results instead of an error
ilya-biryukov marked 2 inline comments as done.Jun 7 2019, 9:08 AM
ilya-biryukov added inline comments.
clang-tools-extra/clangd/ClangdLSPServer.cpp
734 ↗(On Diff #203514)

Returning empty completion results seems to work just fine.
Added a comment anyway, but I don't think we need an LSP bug for this, returning empty results seems less cheesy than a 'request cancelled' error.

ilya-biryukov edited the summary of this revision. (Show Details)Jun 7 2019, 9:10 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2019, 9:21 AM