https://github.com/HighCommander4 requested changes to this pull request.
I haven't had a chance to take a very detailed look, but some high-level feedback: * We shouldn't assume the current working directory (`current_path`) will be the workspace root, even if that happens to be the case today. Instead, let's propagate the `rootUri` (or `rootPath`) that the client sent in the `initialize` request to the place where we need it. We already propagate it into [`Opts.WorkspaceRoot`](https://searchfox.org/llvm/rev/d90bc3bc609d3ef2254e85cfcd435a99eb2b019b/clang-tools-extra/clangd/ClangdLSPServer.cpp#511-514), we can plumb it further from there into `GlobalCompilationDatabase`. * Rather than changing the `GlobalCompilationDatabase::getFallbackCommand` signature, let's provide the flag to the `GlobalCompilationDatabase` via its constructor (and derived classes' constructors). * In fact, since we need to tell it both the bool and the path, maybe we can group them into an `optional<string>`, which would contain the path in strong workspace mode, and be empty in the default mode. * We should add some sort of test coverage. We can add a unit test similar to [`GlobalCompilationDatabaseTest.FallbackCommand`](https://searchfox.org/llvm/rev/d90bc3bc609d3ef2254e85cfcd435a99eb2b019b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp#41-56) which makes assertions about `Cmd.Directory`. https://github.com/llvm/llvm-project/pull/155905 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
