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