hokein added inline comments.
================ Comment at: clangd/ClangdLSPServer.h:132 - RealFileSystemProvider FSProvider; /// Options used for code completion ---------------- sammccall wrote: > ilya-biryukov wrote: > > Could we instead call `getRealFS()` when we try to initialize a clang-tidy > > options provider in `main()` and avoid changing this? > > To avoid adding extra non-real-fs "modes of operation" to > > `ClangdLSPServer`. Unless you see other uses for this. > We already have out-of-tree modifications to ClangdLSPServer to use non-real > FSes. > Given that, I think this change is OK... though better still might be to move > it into `ClangdServer::Options` Yes, this is the main reason I did this change. ================ Comment at: clangd/ClangdUnit.h:83 + IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, + tidy::ClangTidyOptions ClangTidyOpts); ---------------- sammccall wrote: > Passing by value is OK here if deliberate, but let's try to avoid too many > random copies below. changed to `const &` ================ Comment at: clangd/tool/ClangdMain.cpp:204 +static llvm::cl::opt<std::string> ClangTidyChecks( + "clang-tidy-checks", ---------------- sammccall wrote: > Maybe add a TODO or FIXME to respect .clang-tidy files? didn't get the point of the comment -- in this patch, clangd will read configurations from `.clang-tidy` files (`FileOptionsProvider` provides this functionality). This command-line flag is used to overwrite the `.clang-tidy` configurations,. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55256/new/ https://reviews.llvm.org/D55256 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
