This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Add a test URI scheme for lit tests to unbreak platform-specific URI failures.
ClosedPublic

Authored by ioeric on Jan 31 2018, 6:35 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ioeric created this revision.Jan 31 2018, 6:35 AM
sammccall accepted this revision.Jan 31 2018, 7:44 AM

Thanks for fixing this once and for all!

clangd/ClangdLSPServer.cpp
23 ↗(On Diff #132167)

nit: URi -> URI

28 ↗(On Diff #132167)

nit: just inline this at the registry site? we shouldn't be using it elsewhere

47 ↗(On Diff #132167)

on windows you're passing C:\clangd-test and /foo.cpp here.
I think you should probably call native on the body before passing it here (and no need to do it after)

clangd/Protocol.cpp
36 ↗(On Diff #132167)

I'd suggest not mentioning 'test' in the log message, and just making this a comment.
If someone passes e.g. http://, mentioning test probably adds more confusion than it resolves.

This revision is now accepted and ready to land.Jan 31 2018, 7:44 AM
ioeric updated this revision to Diff 132180.Jan 31 2018, 8:27 AM
ioeric marked 4 inline comments as done.
  • Addressed review comments.
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp