keith created this revision.
Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman, hiraditya.
keith requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, MaskRay, ilya-biryukov.
Herald added projects: LLVM, clang-tools-extra.

It's common for CLIs on macOS to read configuration files from more
unix-standard directories with either dotfiles directly in ~ or files in
~/.config. Previously macOS clangd configuration only read from
~/Library/Preferences on macOS, now it respects the other Unix
directories with XDG_CONFIG_HOME or just ~/.config as a fallback. This
is implemented by changing user_config_directory to
user_config_directories which sets a vector of paths to search,
maintaining the previous order of operations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113645

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Unix/Path.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===================================================================
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -509,9 +509,10 @@
 TEST(Support, ConfigDirectoryWithEnv) {
   WithEnv Env("XDG_CONFIG_HOME", "/xdg/config");
 
-  SmallString<128> ConfigDir;
-  EXPECT_TRUE(path::user_config_directory(ConfigDir));
-  EXPECT_EQ("/xdg/config", ConfigDir);
+  std::vector<std::string> ConfigDirs;
+  EXPECT_TRUE(path::user_config_directories(ConfigDirs));
+  std::vector<std::string> Expected = {"/xdg/config"};
+  EXPECT_EQ(Expected, ConfigDirs);
 }
 
 TEST(Support, ConfigDirectoryNoEnv) {
@@ -521,9 +522,10 @@
   ASSERT_TRUE(path::home_directory(Fallback));
   path::append(Fallback, ".config");
 
-  SmallString<128> CacheDir;
-  EXPECT_TRUE(path::user_config_directory(CacheDir));
-  EXPECT_EQ(Fallback, CacheDir);
+  std::vector<std::string> ConfigDirs;
+  EXPECT_TRUE(path::user_config_directories(ConfigDirs));
+  std::vector<std::string> Expected = {std::string(Fallback)};
+  EXPECT_EQ(Expected, ConfigDirs);
 }
 
 TEST(Support, CacheDirectoryWithEnv) {
@@ -549,13 +551,41 @@
 
 #ifdef __APPLE__
 TEST(Support, ConfigDirectory) {
-  SmallString<128> Fallback;
-  ASSERT_TRUE(path::home_directory(Fallback));
-  path::append(Fallback, "Library/Preferences");
+  WithEnv Env("XDG_CONFIG_HOME", nullptr);
 
-  SmallString<128> ConfigDir;
-  EXPECT_TRUE(path::user_config_directory(ConfigDir));
-  EXPECT_EQ(Fallback, ConfigDir);
+  SmallString<128> HomerDir;
+  ASSERT_TRUE(path::home_directory(HomerDir));
+
+  SmallString<128> Preferences = HomerDir;
+  path::append(Preferences, "Library/Preferences");
+
+  SmallString<128> ConfigDir = HomerDir;
+  path::append(ConfigDir, ".config");
+
+  std::vector<std::string> ConfigDirs;
+  EXPECT_TRUE(path::user_config_directories(ConfigDirs));
+  std::vector<std::string> Expected = {std::string(Preferences),
+                                       std::string(ConfigDir)};
+  EXPECT_TRUE(Expected == ConfigDirs);
+}
+
+TEST(Support, XDGConfigDirectory) {
+  WithEnv Env("XDG_CONFIG_HOME", "/xdg/config");
+
+  SmallString<128> HomerDir;
+  ASSERT_TRUE(path::home_directory(HomerDir));
+
+  SmallString<128> Preferences = HomerDir;
+  path::append(Preferences, "Library/Preferences");
+
+  SmallString<128> ConfigDir = HomerDir;
+  path::append(ConfigDir, ".config");
+
+  std::vector<std::string> ConfigDirs;
+  EXPECT_TRUE(path::user_config_directories(ConfigDirs));
+  std::vector<std::string> Expected = {std::string(Preferences), "/xdg/config",
+                                       std::string(ConfigDir)};
+  EXPECT_TRUE(Expected == ConfigDirs);
 }
 #endif
 
Index: llvm/lib/Support/Windows/Path.inc
===================================================================
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -1434,10 +1434,13 @@
   return getKnownFolderPath(FOLDERID_Profile, result);
 }
 
-bool user_config_directory(SmallVectorImpl<char> &result) {
+bool user_config_directories(std::vector<std::string> &results) {
   // Either local or roaming appdata may be suitable in some cases, depending
   // on the data. Local is more conservative, Roaming may not always be correct.
-  return getKnownFolderPath(FOLDERID_LocalAppData, result);
+  SmallString<MAX_PATH> result;
+  bool found = getKnownFolderPath(FOLDERID_LocalAppData, &result);
+  results.push_back(result.c_str());
+  return found;
 }
 
 bool cache_directory(SmallVectorImpl<char> &result) {
Index: llvm/lib/Support/Unix/Path.inc
===================================================================
--- llvm/lib/Support/Unix/Path.inc
+++ llvm/lib/Support/Unix/Path.inc
@@ -1363,27 +1363,34 @@
   return false;
 }
 
-bool user_config_directory(SmallVectorImpl<char> &result) {
+bool user_config_directories(std::vector<std::string> &results) {
+  bool found = false;
+  SmallString<128> result;
 #ifdef __APPLE__
   // Mac: ~/Library/Preferences/
   if (home_directory(result)) {
     append(result, "Library", "Preferences");
-    return true;
+    results.push_back(result.c_str());
+    found = true;
   }
-#else
+#endif
+
   // XDG_CONFIG_HOME as defined in the XDG Base Directory Specification:
   // http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
   if (const char *RequestedDir = getenv("XDG_CONFIG_HOME")) {
     result.clear();
     result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
-    return true;
+    results.push_back(result.c_str());
+    found = true;
   }
-#endif
+
   // Fallback: ~/.config
+  result.clear();
   if (!home_directory(result)) {
-    return false;
+    return found;
   }
   append(result, ".config");
+  results.push_back(result.c_str());
   return true;
 }
 
Index: llvm/include/llvm/Support/Path.h
===================================================================
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ -419,12 +419,12 @@
 /// @result True if a home directory is set, false otherwise.
 bool home_directory(SmallVectorImpl<char> &result);
 
-/// Get the directory where packages should read user-specific configurations.
-/// e.g. $XDG_CONFIG_HOME.
+/// Get the directories where packages should read user-specific
+/// configurations. e.g. $XDG_CONFIG_HOME.
 ///
-/// @param result Holds the resulting path name.
+/// @param result Holds the resulting paths.
 /// @result True if the appropriate path was determined, it need not exist.
-bool user_config_directory(SmallVectorImpl<char> &result);
+bool user_config_directories(std::vector<std::string> &results);
 
 /// Get the directory where installed packages should put their
 /// machine-local cache, e.g. $XDG_CACHE_HOME.
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -892,12 +892,16 @@
   if (EnableConfig) {
     ProviderStack.push_back(
         config::Provider::fromAncestorRelativeYAMLFiles(".clangd", TFS));
-    llvm::SmallString<256> UserConfig;
-    if (llvm::sys::path::user_config_directory(UserConfig)) {
-      llvm::sys::path::append(UserConfig, "clangd", "config.yaml");
-      vlog("User config file is {0}", UserConfig);
-      ProviderStack.push_back(config::Provider::fromYAMLFile(
-          UserConfig, /*Directory=*/"", TFS, /*Trusted=*/true));
+    std::vector<std::string> UserConfigs;
+    if (llvm::sys::path::user_config_directories(UserConfigs)) {
+      for (auto ConfigRoot : UserConfigs) {
+        llvm::SmallString<128> UserConfig;
+        UserConfig.assign(ConfigRoot);
+        llvm::sys::path::append(UserConfig, "clangd", "config.yaml");
+        vlog("User config file is {0}", UserConfig);
+        ProviderStack.push_back(config::Provider::fromYAMLFile(
+            UserConfig, /*Directory=*/"", TFS, /*Trusted=*/true));
+      }
     } else {
       elog("Couldn't determine user config file, not loading");
     }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to