change: default config location on macOS considers XDG config var (#1570

)

Actually support $XDG_CONFIG_HOME on macOS. Apparently in our docs we also say we do, but we, uh, don't, because dirs doesn't.

Note this is backwards-compatible, in that if a config file exists in the old default locations, we will check those first.

CHANGELOG.md

@@ -30,6 +30,8 @@ That said, these are more guidelines rather than hardset rules, though the proje
 ### Changes
 
 - [#1559](https://github.com/ClementTsang/bottom/pull/1559): Rename `--enable_gpu` to `--disable_gpu`, and make GPU features enabled by default.
+- [#1570](https://github.com/ClementTsang/bottom/pull/1570): Consider `$XDG_CONFIG_HOME` on macOS when looking for a default config path in a
+  backwards-compatible fashion.
 
 ## [0.10.2] - 2024-08-05
 

docs/content/configuration/config-file/index.md

@@ -6,11 +6,11 @@ For persistent configuration, and for certain configuration options, bottom supp
 
 If no config file argument is given, it will automatically look for a config file at these locations:
 
-| OS      | Default Config Location                                                                                                                |
-| ------- | -------------------------------------------------------------------------------------------------------------------------------------- |
-| macOS   | `$HOME/Library/Application Support/bottom/bottom.toml`<br/> `~/.config/bottom/bottom.toml` <br/> `$XDG_CONFIG_HOME/bottom/bottom.toml` |
-| Linux   | `~/.config/bottom/bottom.toml` <br/> `$XDG_CONFIG_HOME/bottom/bottom.toml`                                                             |
-| Windows | `C:\Users\<USER>\AppData\Roaming\bottom\bottom.toml`                                                                                   |
+| OS      | Default Config Location                                                                                                                    |
+| ------- | ------------------------------------------------------------------------------------------------------------------------------------------ |
+| macOS   | `$HOME/Library/Application Support/bottom/bottom.toml`<br/> `$HOME/.config/bottom/bottom.toml` <br/> `$XDG_CONFIG_HOME/bottom/bottom.toml` |
+| Linux   | `$HOME/.config/bottom/bottom.toml` <br/> `$XDG_CONFIG_HOME/bottom/bottom.toml`                                                             |
+| Windows | `C:\Users\<USER>\AppData\Roaming\bottom\bottom.toml`                                                                                       |
 
 If the config file doesn't exist at the path, bottom will automatically try to create a new config file at the location
 with default values.

src/options.rs

@@ -60,36 +60,67 @@ macro_rules! is_flag_enabled {
     };
 }
 
+/// The default config file sub-path.
+const DEFAULT_CONFIG_FILE_LOCATION: &str = "bottom/bottom.toml";
+
 /// Returns the config path to use. If `override_config_path` is specified, then
 /// we will use that. If not, then return the "default" config path, which is:
+///
 /// - If a path already exists at `<HOME>/bottom/bottom.toml`, then use that for
 ///   legacy reasons.
 /// - Otherwise, use `<SYSTEM_CONFIG_FOLDER>/bottom/bottom.toml`.
 ///
 /// For more details on this, see [dirs](https://docs.rs/dirs/latest/dirs/fn.config_dir.html)'
 /// documentation.
+///
+/// XXX: For macOS, we additionally will manually check `$XDG_CONFIG_HOME` as well first
+/// before falling back to `dirs`.
 fn get_config_path(override_config_path: Option<&Path>) -> Option<PathBuf> {
-    const DEFAULT_CONFIG_FILE_PATH: &str = "bottom/bottom.toml";
-
     if let Some(conf_loc) = override_config_path {
         return Some(conf_loc.to_path_buf());
     } else if let Some(home_path) = dirs::home_dir() {
         let mut old_home_path = home_path;
         old_home_path.push(".config/");
-        old_home_path.push(DEFAULT_CONFIG_FILE_PATH);
-        if old_home_path.exists() {
-            // We used to create it at `<HOME>/DEFAULT_CONFIG_FILE_PATH`, but changed it
-            // to be more correct later. However, for legacy reasons, if it already exists,
-            // use the old one.
-            return Some(old_home_path);
+        old_home_path.push(DEFAULT_CONFIG_FILE_LOCATION);
+        if let Ok(res) = old_home_path.try_exists() {
+            if res {
+                // We used to create it at `<HOME>/DEFAULT_CONFIG_FILE_PATH`, but changed it
+                // to be more correct later. However, for legacy reasons, if it already exists,
+                // use the old one.
+                return Some(old_home_path);
+            }
         }
     }
 
-    // Otherwise, return the "correct" path based on the config dir.
-    dirs::config_dir().map(|mut path| {
-        path.push(DEFAULT_CONFIG_FILE_PATH);
+    let config_path = dirs::config_dir().map(|mut path| {
+        path.push(DEFAULT_CONFIG_FILE_LOCATION);
         path
-    })
+    });
+
+    if cfg!(target_os = "macos") {
+        if let Ok(xdg_config_path) = std::env::var("XDG_CONFIG_HOME") {
+            if !xdg_config_path.is_empty() {
+                // If XDG_CONFIG_HOME exists and is non-empty, _but_ we previously used the Library-based path
+                // for a config and it exists, then use that instead for backwards-compatibility.
+                if let Some(old_macos_path) = &config_path {
+                    if let Ok(res) = old_macos_path.try_exists() {
+                        if res {
+                            return config_path;
+                        }
+                    }
+                }
+
+                // Otherwise, try and use the XDG_CONFIG_HOME-based path.
+                let mut cfg_path = PathBuf::new();
+                cfg_path.push(xdg_config_path);
+                cfg_path.push(DEFAULT_CONFIG_FILE_LOCATION);
+
+                return Some(cfg_path);
+            }
+        }
+    }
+
+    config_path
 }
 
 fn create_config_at_path(path: &Path) -> anyhow::Result<Config> {
@@ -1183,4 +1214,47 @@ mod test {
             }
         }
     }
+
+    /// This one has slightly more complex behaviour due to `dirs` not respecting XDG on macOS, so we manually
+    /// handle it. However, to ensure backwards-compatibility, we also have to do some special cases.
+    #[cfg(target_os = "macos")]
+    #[test]
+    fn test_get_config_path_macos() {
+        use super::get_config_path;
+        use super::DEFAULT_CONFIG_FILE_LOCATION;
+        use std::path::PathBuf;
+
+        // Case three: no previous config, no XDG var.
+        // SAFETY: this is the only test that does this
+        unsafe {
+            std::env::remove_var("XDG_CONFIG_HOME");
+        }
+
+        let case_1 = dirs::config_dir()
+            .map(|mut path| {
+                path.push(DEFAULT_CONFIG_FILE_LOCATION);
+                path
+            })
+            .unwrap();
+
+        // Skip this test if the file already exists.
+        if !case_1.exists() {
+            assert_eq!(get_config_path(None), Some(case_1));
+        }
+
+        // Case two: no previous config, XDG var exists.
+        std::env::set_var("XDG_CONFIG_HOME", "/tmp");
+        let mut case_2 = PathBuf::new();
+        case_2.push("/tmp");
+        case_2.push(DEFAULT_CONFIG_FILE_LOCATION);
+
+        // Skip this test if the file already exists.
+        if !case_2.exists() {
+            assert_eq!(get_config_path(None), Some(case_2));
+        }
+
+        // Case one: old non-XDG exists already, XDG var exists.
+        // let case_3 = case_1;
+        // assert_eq!(get_config_path(None), Some(case_1));
+    }
 }

src/options/config/style.rs

@@ -231,7 +231,6 @@ mod test {
     #[test]
     fn default_selected_colour_works() {
         let mut colours = ColourPalette::default();
-        println!("colours: {colours:?}");
         let original_selected_text_colour = ColourPalette::default_palette()
             .selected_text_style
             .fg