[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 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Thilo Vörtler via Phabricator via cfe-commits]

voertler added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Thilo Vörtler via Phabricator via cfe-commits]

voertler added a reviewer: sammccall.
voertler added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Thilo Vörtler via Phabricator via cfe-commits]

voertler created this revision.
voertler added reviewers: dgoldman, kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
voertler requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This patch allows it to set a default configuration as discussed in issue 
https://github.com/clangd/clangd/issues/845 by adding a startup option 
--default-config.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151315

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -317,7 +317,6 @@
 RetiredFlag InlayHints("inlay-hints");
 RetiredFlag FoldingRanges("folding-ranges");
 
-
 opt LimitResults{
 "limit-results",
 cat(Features),
@@ -497,7 +496,8 @@
 desc(
 "Read user and project configuration from YAML files.\n"
 "Project config is from a .clangd file in the project directory.\n"
-"User config is from clangd/config.yaml in the following 
directories:\n"
+"User config is from clangd/config.yaml in the following directories 
\n"
+"(unless specified with --default-config):\n"
 "\tWindows: %USERPROFILE%\\AppData\\Local\n"
 "\tMac OS: ~/Library/Preferences/\n"
 "\tOthers: $XDG_CONFIG_HOME, usually ~/.config\n"
@@ -505,6 +505,15 @@
 init(true),
 };
 
+opt DefaultConfig{
+"default-config",
+cat(Misc),
+desc("Path to a default clangd configuration file. A clangd user and "
+ "project configuration has a higher priority (requires "
+ "--enable-config) "),
+init(""),
+};
+
 opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
   desc("Use files open in the editor when parsing "
"headers instead of reading from the disk"),
@@ -945,6 +954,21 @@
 } else {
   elog("Couldn't determine user config file, not loading");
 }
+if (DefaultConfig.getNumOccurrences()) {
+  llvm::SmallString<256> DefaultConfigPath;
+  if (auto Error = llvm::sys::fs::real_path(
+  DefaultConfig, DefaultConfigPath, /*expand_tilde=*/true)) {
+elog("Couldn't determine default config file {0}: {1} , not loading",
+ DefaultConfig, Error.message());
+  } else {
+vlog("Default config file is {0}", DefaultConfigPath);
+// Give lowest priority to default config 
+ProviderStack.insert(
+ProviderStack.begin(),
+config::Provider::fromYAMLFile(DefaultConfigPath, /*Directory=*/"",
+   TFS, /*Trusted=*/true));
+  }
+}
   }
   ProviderStack.push_back(std::make_unique());
   std::vector ProviderPointers;


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -317,7 +317,6 @@
 RetiredFlag InlayHints("inlay-hints");
 RetiredFlag FoldingRanges("folding-ranges");
 
-
 opt LimitResults{
 "limit-results",
 cat(Features),
@@ -497,7 +496,8 @@
 desc(
 "Read user and project configuration from YAML files.\n"
 "Project config is from a .clangd file in the project directory.\n"
-"User config is from clangd/config.yaml in the following directories:\n"
+"User config is from clangd/config.yaml in the following directories \n"
+"(unless specified with --default-config):\n"
 "\tWindows: %USERPROFILE%\\AppData\\Local\n"
 "\tMac OS: ~/Library/Preferences/\n"
 "\tOthers: $XDG_CONFIG_HOME, usually ~/.config\n"
@@ -505,6 +505,15 @@
 init(true),
 };
 
+opt DefaultConfig{
+"default-config",
+cat(Misc),
+desc("Path to a default clangd configuration file. A clangd user and "
+ "project configuration has a higher priority (requires "
+ "--enable-config) "),
+init(""),
+};
+
 opt UseDirtyHeaders{"use-dirty-headers", cat(Misc),
   desc("Use files open in the editor when parsing "
"headers instead of reading from the disk"),
@@ -945,6 +954,21 @@
 } else {
   elog("Couldn't determine user config file, not loading");
 }
+if (DefaultConfig.getNumOccurrences()) {
+  llvm::SmallString<256> DefaultConfigPath;
+  if (auto Error = llvm::sys::fs::real_path(
+  DefaultConfig, DefaultConfigPath, /*expand_tilde=*/true)) {
+elog("Couldn't determine default config file {0}: {1} , not loading",
+ DefaultConfig, Error.message());
+  } else {
+vlog("Default config file is {0}", DefaultConfigPath);
+// Give lowest priority to default config 
+