[PATCH] D151315: [clangd] Add a switch to specify a default clangd configuration file

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry about the delay, I think this is OK to add.



================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:500
+        "User config is from clangd/config.yaml in the following directories \n"
+        "(unless specified with --default-config):\n"
         "\tWindows: %USERPROFILE%\\AppData\\Local\n"
----------------
why this change? user config is still loaded from those directories

Maybe after the user config text, add "Extra config may be specified by --extra-config-file"


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:509
+opt<std::string> DefaultConfig{
+    "default-config",
+    cat(Misc),
----------------
"default" doesn't seem like the right term for this, as it's clearly not the default - the user is providing it!

Also, there's also been desire to provide config inline in a flag (as opposed to a file) as this is more self-contained. Calling this config when it's actually a config filename is a bit confusing.

maybe `--extra-config-file`?


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:512
+    desc("Path to a default clangd configuration file. A clangd user and "
+         "project configuration has a higher priority (requires "
+         "--enable-config) "),
----------------
I think mentioning --enable-config is more confusing than helpful, since it's on by default.


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:959
+      llvm::SmallString<256> DefaultConfigPath;
+      if (auto Error = llvm::sys::fs::real_path(
+              DefaultConfig, DefaultConfigPath, /*expand_tilde=*/true)) {
----------------
none of this path checking/resolution should be necessary, the fromYAMLFile provider already handles the case where the path does not exist.

We don't expand tildes in paths clangd (or clang, generally) processes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151315/new/

https://reviews.llvm.org/D151315