Page MenuHomePhabricator

[clangd] Support passing a relative path to -compile-commands-dir
ClosedPublic

Authored by DaanDeMeyer on Oct 21 2018, 10:26 AM.

Details

Summary

This is useful when using clangd with CMake based projects in Visual Studio Code since when using CMake the compile_commands.json file is usually located in a build subdirectory which isn't a parent directory of the source files. Allowing passing relative paths to -compile-commands-dir allows specifying clangd.arguments = ["-compile-commands-dir=build"] in VSCode's settings file and having it work for each CMake based project that uses the build subdirectory as the build directory (instead of having to specify the absolute path to the compile commands directory for each separate project in VSCode's settings).

Diff Detail

Repository
rL LLVM

Event Timeline

DaanDeMeyer created this revision.Oct 21 2018, 10:26 AM
DaanDeMeyer removed a reviewer: Restricted Project.Oct 21 2018, 10:28 AM
sammccall accepted this revision.Oct 21 2018, 11:49 PM

Thanks for the fix!

clangd/tool/ClangdMain.cpp
261 ↗(On Diff #170338)

This comment echoes the code, but doesn't say *why*.
Consider replacing with
// clangd changes working directory a lot, so we need an absolute path.

263 ↗(On Diff #170338)

it's fine to call make_absolute on an absolute path, it's a no-op.
You can do that here to avoid a special case.

267 ↗(On Diff #170338)

nit: we tend to inline this as if (std::error_code EC = sys::fs...)

272 ↗(On Diff #170338)

This is a no-op (as are the existing ones on 255 and 259...)

This revision is now accepted and ready to land.Oct 21 2018, 11:49 PM

Updated diff according to comments.

DaanDeMeyer marked 4 inline comments as done.Oct 22 2018, 3:56 AM

It looks like these are your first couple of LLVM patches.
So, welcome :-) And do you want me to commit them for you?

Typically your first few patches get landed by someone else, and then you ask for commit access: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

You can commit them. Thanks for all the quick responses!

This revision was automatically updated to reflect the committed changes.