From beb71fb69b0918d13984ff5af771579ff055ff86 Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Sun, 27 Apr 2025 15:44:18 -0400 Subject: [PATCH 01/12] Initial Implementation: Create a set of potential config locations to search for --- src/args.rs | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/args.rs b/src/args.rs index 4c1a64ec92..3c0e56ac4a 100644 --- a/src/args.rs +++ b/src/args.rs @@ -159,15 +159,42 @@ fn get_app_cache_path() -> Result { } pub fn get_app_config_path() -> Result { - let mut path = if cfg!(target_os = "macos") { - dirs::home_dir().map(|h| h.join(".config")) - } else { - dirs::config_dir() + // List of potential config directories in order of priority + let potential_config_dirs = [ + env::var_os("XDG_CONFIG_HOME").map(PathBuf::from), + // This is in the list since it was the hardcoded behavior on macos before + // I expect this to be what most people have XDG_CONFIG_HOME set to already + // But explicitly including this will avoid breaking anyone's existing config + dirs::home_dir().map(|p| p.join(".config")), + dirs::config_dir(), + ] + .into_iter() + // Remove any that resulted in None from unset env vars + .flatten() + // Remove any paths that aren't found on the system + .filter(|path| path.is_dir()); + + let mut target_config_dir = None; + + for potential_dir in potential_config_dirs { + let search_path = potential_dir.join("gitui"); + + // Prefer any preexisting gitui config dir + if search_path.is_dir() { + target_config_dir = Some(search_path); + break; + } + + // Set fallback to first existing directory + if target_config_dir.is_none() { + target_config_dir = Some(search_path); + } } - .ok_or_else(|| anyhow!("failed to find os config dir."))?; - path.push("gitui"); - Ok(path) + let config_dir = target_config_dir + .ok_or_else(|| anyhow!("failed to find os cache dir."))?; + + Ok(config_dir) } #[test] From 8e307d30ec9622be121de5d616370b19064cb99a Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Mon, 28 Apr 2025 02:16:51 -0400 Subject: [PATCH 02/12] Apply the same logic to get_app_cache_path --- src/args.rs | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/args.rs b/src/args.rs index 3c0e56ac4a..4d579a6df9 100644 --- a/src/args.rs +++ b/src/args.rs @@ -150,12 +150,36 @@ fn setup_logging(path_override: Option) -> Result<()> { } fn get_app_cache_path() -> Result { - let mut path = dirs::cache_dir() + let potential_cache_dirs = [ + env::var_os("XDG_CACHE_HOME").map(PathBuf::from), + dirs::cache_dir(), + ] + .into_iter() + .flatten() + .filter(|path| path.is_dir()); + + let mut target_cache_dir = None; + + for potential_dir in potential_cache_dirs { + let search_path = potential_dir.join("gitui"); + + // Prefer any preexisting gitui cache dir + if search_path.is_dir() { + target_cache_dir = Some(search_path); + break; + } + + // Set fallback to first existing directory + if target_cache_dir.is_none() { + target_cache_dir = Some(search_path); + } + } + + let cache_dir = target_cache_dir .ok_or_else(|| anyhow!("failed to find os cache dir."))?; - path.push("gitui"); - fs::create_dir_all(&path)?; - Ok(path) + fs::create_dir_all(&cache_dir)?; + Ok(cache_dir) } pub fn get_app_config_path() -> Result { From fb935b71fa33ae2b7a4b74163d4529fbacf1f3ef Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Mon, 28 Apr 2025 05:09:03 -0400 Subject: [PATCH 03/12] Refactor to make logic more reusable and testable --- src/args.rs | 82 ++++++++++++++++++++++------------------------------- 1 file changed, 34 insertions(+), 48 deletions(-) diff --git a/src/args.rs b/src/args.rs index 4d579a6df9..008f808b23 100644 --- a/src/args.rs +++ b/src/args.rs @@ -149,76 +149,62 @@ fn setup_logging(path_override: Option) -> Result<()> { Ok(()) } -fn get_app_cache_path() -> Result { - let potential_cache_dirs = [ - env::var_os("XDG_CACHE_HOME").map(PathBuf::from), - dirs::cache_dir(), - ] - .into_iter() - .flatten() - .filter(|path| path.is_dir()); +fn get_path_from_canidates( + canidates: Vec>, +) -> Result { + let valid_canidates = + canidates.into_iter().flatten().filter(|path| path.is_dir()); - let mut target_cache_dir = None; + let mut target_dir = None; - for potential_dir in potential_cache_dirs { + for potential_dir in valid_canidates { let search_path = potential_dir.join("gitui"); - // Prefer any preexisting gitui cache dir + // Prefer preexisting gitui directory if search_path.is_dir() { - target_cache_dir = Some(search_path); + target_dir = Some(search_path); break; } - // Set fallback to first existing directory - if target_cache_dir.is_none() { - target_cache_dir = Some(search_path); - } + // Fallback to first existing directory + target_dir.get_or_insert(search_path); } - let cache_dir = target_cache_dir - .ok_or_else(|| anyhow!("failed to find os cache dir."))?; + let dir = target_dir.ok_or_else(|| { + anyhow!("failed to find valid path within canidates") + })?; + + Ok(dir) +} + +fn get_app_cache_path() -> Result { + let cache_dir_canidates = vec![ + env::var_os("XDG_CACHE_HOME").map(PathBuf::from), + dirs::cache_dir(), + ]; - fs::create_dir_all(&cache_dir)?; - Ok(cache_dir) + match get_path_from_canidates(cache_dir_canidates) { + Ok(cache_dir) => { + fs::create_dir_all(&cache_dir)?; + Ok(cache_dir) + } + Err(_) => Err(anyhow!("failed to find os cache dir.")), + } } pub fn get_app_config_path() -> Result { // List of potential config directories in order of priority - let potential_config_dirs = [ + let config_dir_canidates = vec![ env::var_os("XDG_CONFIG_HOME").map(PathBuf::from), // This is in the list since it was the hardcoded behavior on macos before // I expect this to be what most people have XDG_CONFIG_HOME set to already // But explicitly including this will avoid breaking anyone's existing config dirs::home_dir().map(|p| p.join(".config")), dirs::config_dir(), - ] - .into_iter() - // Remove any that resulted in None from unset env vars - .flatten() - // Remove any paths that aren't found on the system - .filter(|path| path.is_dir()); - - let mut target_config_dir = None; - - for potential_dir in potential_config_dirs { - let search_path = potential_dir.join("gitui"); - - // Prefer any preexisting gitui config dir - if search_path.is_dir() { - target_config_dir = Some(search_path); - break; - } - - // Set fallback to first existing directory - if target_config_dir.is_none() { - target_config_dir = Some(search_path); - } - } - - let config_dir = target_config_dir - .ok_or_else(|| anyhow!("failed to find os cache dir."))?; + ]; - Ok(config_dir) + get_path_from_canidates(config_dir_canidates) + .map_err(|_| anyhow!("failed to find os config dir.")) } #[test] From 819b4ca5bbf701db5565bfc0309e776b448de28f Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Mon, 28 Apr 2025 13:35:03 -0400 Subject: [PATCH 04/12] Realize I don't know how to spell candidates --- src/args.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/args.rs b/src/args.rs index 008f808b23..f5f56366c2 100644 --- a/src/args.rs +++ b/src/args.rs @@ -149,15 +149,17 @@ fn setup_logging(path_override: Option) -> Result<()> { Ok(()) } -fn get_path_from_canidates( - canidates: Vec>, +fn get_path_from_candidates( + candidates: Vec>, ) -> Result { - let valid_canidates = - canidates.into_iter().flatten().filter(|path| path.is_dir()); + let valid_candidates = candidates + .into_iter() + .flatten() + .filter(|path| path.is_dir()); let mut target_dir = None; - for potential_dir in valid_canidates { + for potential_dir in valid_candidates { let search_path = potential_dir.join("gitui"); // Prefer preexisting gitui directory @@ -171,19 +173,19 @@ fn get_path_from_canidates( } let dir = target_dir.ok_or_else(|| { - anyhow!("failed to find valid path within canidates") + anyhow!("failed to find valid path within candidates") })?; Ok(dir) } fn get_app_cache_path() -> Result { - let cache_dir_canidates = vec![ + let cache_dir_candidates = vec![ env::var_os("XDG_CACHE_HOME").map(PathBuf::from), dirs::cache_dir(), ]; - match get_path_from_canidates(cache_dir_canidates) { + match get_path_from_candidates(cache_dir_candidates) { Ok(cache_dir) => { fs::create_dir_all(&cache_dir)?; Ok(cache_dir) @@ -194,7 +196,7 @@ fn get_app_cache_path() -> Result { pub fn get_app_config_path() -> Result { // List of potential config directories in order of priority - let config_dir_canidates = vec![ + let config_dir_candidates = vec![ env::var_os("XDG_CONFIG_HOME").map(PathBuf::from), // This is in the list since it was the hardcoded behavior on macos before // I expect this to be what most people have XDG_CONFIG_HOME set to already @@ -203,7 +205,7 @@ pub fn get_app_config_path() -> Result { dirs::config_dir(), ]; - get_path_from_canidates(config_dir_canidates) + get_path_from_candidates(config_dir_candidates) .map_err(|_| anyhow!("failed to find os config dir.")) } From cbc388d78b5812e42838dda4b1c87e3cecbdc65b Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Mon, 28 Apr 2025 13:50:21 -0400 Subject: [PATCH 05/12] refactor to be a little cleaner --- src/args.rs | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/args.rs b/src/args.rs index f5f56366c2..c2a870ffff 100644 --- a/src/args.rs +++ b/src/args.rs @@ -150,16 +150,14 @@ fn setup_logging(path_override: Option) -> Result<()> { } fn get_path_from_candidates( - candidates: Vec>, + candidates: impl IntoIterator>, ) -> Result { - let valid_candidates = candidates - .into_iter() - .flatten() - .filter(|path| path.is_dir()); - let mut target_dir = None; - for potential_dir in valid_candidates { + // Filter into existing directories + for potential_dir in + candidates.into_iter().flatten().filter(|p| p.is_dir()) + { let search_path = potential_dir.join("gitui"); // Prefer preexisting gitui directory @@ -172,31 +170,27 @@ fn get_path_from_candidates( target_dir.get_or_insert(search_path); } - let dir = target_dir.ok_or_else(|| { + target_dir.ok_or_else(|| { anyhow!("failed to find valid path within candidates") - })?; - - Ok(dir) + }) } fn get_app_cache_path() -> Result { - let cache_dir_candidates = vec![ + let cache_dir_candidates = [ env::var_os("XDG_CACHE_HOME").map(PathBuf::from), dirs::cache_dir(), ]; - match get_path_from_candidates(cache_dir_candidates) { - Ok(cache_dir) => { - fs::create_dir_all(&cache_dir)?; - Ok(cache_dir) - } - Err(_) => Err(anyhow!("failed to find os cache dir.")), - } + let cache_dir = get_path_from_candidates(cache_dir_candidates) + .map_err(|_| anyhow!("failed to find os cache dir."))?; + + fs::create_dir_all(&cache_dir)?; + Ok(cache_dir) } pub fn get_app_config_path() -> Result { // List of potential config directories in order of priority - let config_dir_candidates = vec![ + let config_dir_candidates = [ env::var_os("XDG_CONFIG_HOME").map(PathBuf::from), // This is in the list since it was the hardcoded behavior on macos before // I expect this to be what most people have XDG_CONFIG_HOME set to already From 299015a3b3d6b062b925218c8c04a56c0acaf0c5 Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Mon, 28 Apr 2025 16:03:38 -0400 Subject: [PATCH 06/12] don't create an empty config folder I am a bit hesitant about this change since I am unsure if any logic depends on a config folder needing to exist. But as far as I can tell nothing does and creating a empty config folder in someones dotfiles when they don't want any configuration for gitui seems like terrible UX. --- src/args.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/args.rs b/src/args.rs index c2a870ffff..3c85513a35 100644 --- a/src/args.rs +++ b/src/args.rs @@ -49,7 +49,6 @@ pub fn process_cmdline() -> Result { .map_or_else(|| PathBuf::from("theme.ron"), PathBuf::from); let confpath = get_app_config_path()?; - fs::create_dir_all(&confpath)?; let theme = confpath.join(arg_theme); let notify_watcher: bool = From 9ea0d0db24f464f11d3ccc13a761017f01cab56f Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Mon, 28 Apr 2025 13:25:21 -0400 Subject: [PATCH 07/12] Add unit tests --- src/args.rs | 49 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/src/args.rs b/src/args.rs index 3c85513a35..c39afad331 100644 --- a/src/args.rs +++ b/src/args.rs @@ -202,7 +202,50 @@ pub fn get_app_config_path() -> Result { .map_err(|_| anyhow!("failed to find os config dir.")) } -#[test] -fn verify_app() { - app().debug_assert(); +#[cfg(test)] +mod tests { + use std::fs; + + use super::{app, get_path_from_candidates}; + use tempfile::tempdir; + + #[test] + fn verify_app() { + app().debug_assert(); + } + + #[test] + fn test_config_dir_candidates_from_preexisting() { + let temp_dummy_1 = tempdir().expect("should create temp dir"); + let temp_dummy_2 = tempdir().expect("should create temp dir"); + let temp_target = tempdir().expect("should create temp dir"); + let temp_goal = temp_target.path().join("gitui"); + + fs::create_dir_all(&temp_goal) + .expect("should create temp target directory"); + + let candidates = [ + Some(temp_dummy_1.path().to_path_buf()), + Some(temp_target.path().to_path_buf()), + Some(temp_dummy_2.path().to_path_buf()), + ]; + let result = get_path_from_candidates(candidates) + .expect("should find the included target"); + assert_eq!(result, temp_goal); + } + + #[test] + fn test_config_dir_candidates_no_preexisting() { + let temp_dummy_1 = tempdir().expect("should create temp dir"); + let temp_dummy_2 = tempdir().expect("should create temp dir"); + + let candidates = [ + Some(temp_dummy_1.path().to_path_buf()), + Some(temp_dummy_2.path().to_path_buf()), + ]; + + let result = get_path_from_candidates(candidates) + .expect("should return first candidate"); + assert_eq!(result, temp_dummy_1.path().join("gitui")); + } } From cde05b15fbb7615f486eb575be68c66b4d6bb641 Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Mon, 28 Apr 2025 15:12:18 -0400 Subject: [PATCH 08/12] Update docs to reflect new behavior --- KEY_CONFIG.md | 15 ++++++++++----- README.md | 4 ++-- THEMES.md | 21 +++++++++++++-------- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/KEY_CONFIG.md b/KEY_CONFIG.md index 658aad11b6..d43351e023 100644 --- a/KEY_CONFIG.md +++ b/KEY_CONFIG.md @@ -19,12 +19,17 @@ Create a `key_bindings.ron` file like this: ) ``` -The config file format based on the [Ron file format](https://github.com/ron-rs/ron). +The keybinding config file uses the [Ron file format](https://github.com/ron-rs/ron). The location of the file depends on your OS: -* `$HOME/.config/gitui/key_bindings.ron` (mac) -* `$XDG_CONFIG_HOME/gitui/key_bindings.ron` (linux using XDG) -* `$HOME/.config/gitui/key_bindings.ron` (linux) -* `%APPDATA%/gitui/key_bindings.ron` (Windows) +`gitui` will look for an existing `/gitui` in the following order: +* `$XDG_CONFIG_HOME/gitui/` (with `XDG_CONFIG_HOME` set) +* `$HOME/.config/gitui/` +* Default OS Location: + * `$HOME/Library/Application Support/` (mac) + * `$HOME/.config/gitui/` (linux) + * `%APPDATA%/gitui/` (Windows) + +Key bindings are configured in `key_bindings.ron` within your first found `gitui` config folder. See all possible keys to overwrite in gitui: [here](https://github.com/gitui-org/gitui/blob/master/src/keys/key_list.rs#L83) diff --git a/README.md b/README.md index dd4270d6ed..3067111396 100644 --- a/README.md +++ b/README.md @@ -250,9 +250,9 @@ see [FAQs page](./FAQ.md) To run with logging enabled run `gitui -l`. This will log to: - +- With `XDG_CACHE_HOME` set: `$XDG_CACHE_HOME/gitui/gitui.log` +or default to - macOS: `$HOME/Library/Caches/gitui/gitui.log` -- Linux using `XDG`: `$XDG_CACHE_HOME/gitui/gitui.log` - Linux: `$HOME/.cache/gitui/gitui.log` - Windows: `%LOCALAPPDATA%/gitui/gitui.log` diff --git a/THEMES.md b/THEMES.md index 52c235b496..376334d349 100644 --- a/THEMES.md +++ b/THEMES.md @@ -7,14 +7,19 @@ default on light terminal: To change the colors of the default theme you need to add a `theme.ron` file that contains the colors you want to override. Note that you don’t have to specify the full theme anymore (as of 0.23). Instead, it is sufficient to override just the values that you want to differ from their default values. -The file uses the [Ron format](https://github.com/ron-rs/ron) and is located at one of the following paths, depending on your operating system: - -* `$HOME/.config/gitui/theme.ron` (mac) -* `$XDG_CONFIG_HOME/gitui/theme.ron` (linux using XDG) -* `$HOME/.config/gitui/theme.ron` (linux) -* `%APPDATA%/gitui/theme.ron` (Windows) - -Alternatively, you can create a theme in the same directory mentioned above and use it with the `-t` flag followed by the name of the file in the directory. E.g. If you are on linux calling `gitui -t arc.ron`, this will load the theme in `$XDG_CONFIG_HOME/gitui/arc.ron` or `$HOME/.config/gitui/arc.ron`. +The theme file uses the [Ron file format](https://github.com/ron-rs/ron). +The location of the file depends on your OS: +`gitui` will look for an existing `/gitui` in the following order: +* `$XDG_CONFIG_HOME/gitui/` (with `XDG_CONFIG_HOME` set) +* `$HOME/.config/gitui/` +* Default OS Location: + * `$HOME/Library/Application Support/` (mac) + * `$HOME/.config/gitui/` (linux) + * `%APPDATA%/gitui/` (Windows) + +The theme is configured in `theme.ron` within your first found `gitui` config folder. + +Alternatively, you can create a theme in the same directory mentioned above and use it with the `-t` flag followed by the name of the file in the directory. E.g. Calling `gitui -t arc.ron` will load the `arc.ron` theme from your first found `/gitui` config folder using the logic above. Example theme override: From e8440c289ff0b876040b7443a919b6165fe535df Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Tue, 29 Apr 2025 05:47:18 -0400 Subject: [PATCH 09/12] Add changelog item --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e60180cdc5..53799e3457 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * add `use_selection_fg` to theme file to allow customizing selection foreground color [[@Upsylonbare](https://github.com/Upsylonbare)] ([#2515](https://github.com/gitui-org/gitui/pull/2515)) ### Changed +* Respect `XDG_CONFIG_HOME` and `XDG_CACHE_HOME` irrespective of OS [[@KlassyKat](https://github.com/KlassyKat)] ([#1498](https://github.com/gitui-org/gitui/issues/1498)) * improve error messages [[@acuteenvy](https://github.com/acuteenvy)] ([#2617](https://github.com/gitui-org/gitui/pull/2617)) * increase MSRV from 1.70 to 1.81 [[@naseschwarz](https://github.com/naseschwarz)] ([#2094](https://github.com/gitui-org/gitui/issues/2094)) * improve syntax highlighting file detection [[@acuteenvy](https://github.com/acuteenvy)] ([#2524](https://github.com/extrawurst/gitui/pull/2524)) From 4bb12798b6391c5bcc5e8050d2f4d9ac28860362 Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Thu, 1 May 2025 17:00:21 -0400 Subject: [PATCH 10/12] Only consider absolute paths as valid --- src/args.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/args.rs b/src/args.rs index c39afad331..f6b4cb8997 100644 --- a/src/args.rs +++ b/src/args.rs @@ -154,8 +154,10 @@ fn get_path_from_candidates( let mut target_dir = None; // Filter into existing directories - for potential_dir in - candidates.into_iter().flatten().filter(|p| p.is_dir()) + for potential_dir in candidates + .into_iter() + .flatten() + .filter(|p| p.is_dir() && p.is_absolute()) { let search_path = potential_dir.join("gitui"); From bafd07cbebe0dcc9accd850d53559b1b51b7e179 Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Thu, 1 May 2025 17:22:20 -0400 Subject: [PATCH 11/12] Change error message wording --- src/args.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/args.rs b/src/args.rs index f6b4cb8997..5f13a3cee3 100644 --- a/src/args.rs +++ b/src/args.rs @@ -183,7 +183,7 @@ fn get_app_cache_path() -> Result { ]; let cache_dir = get_path_from_candidates(cache_dir_candidates) - .map_err(|_| anyhow!("failed to find os cache dir."))?; + .map_err(|_| anyhow!("failed to find valid cache dir."))?; fs::create_dir_all(&cache_dir)?; Ok(cache_dir) @@ -201,7 +201,7 @@ pub fn get_app_config_path() -> Result { ]; get_path_from_candidates(config_dir_candidates) - .map_err(|_| anyhow!("failed to find os config dir.")) + .map_err(|_| anyhow!("failed to find valid config dir.")) } #[cfg(test)] From 759c26a6bdc63febab492685d865dc0a7ed96bc9 Mon Sep 17 00:00:00 2001 From: KlassyKat Date: Thu, 1 May 2025 21:18:41 -0400 Subject: [PATCH 12/12] Add utility function to ensure select paths exist Might remove this after further scrutinizing the XDG spec --- src/args.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/args.rs b/src/args.rs index 5f13a3cee3..afeb8fb576 100644 --- a/src/args.rs +++ b/src/args.rs @@ -148,6 +148,15 @@ fn setup_logging(path_override: Option) -> Result<()> { Ok(()) } +fn ensure_path_exists(path: Option) -> Option { + path.and_then(|p| { + if p.is_absolute() && fs::create_dir_all(&p).is_ok() { + return Some(p); + } + None + }) +} + fn get_path_from_candidates( candidates: impl IntoIterator>, ) -> Result { @@ -178,7 +187,9 @@ fn get_path_from_candidates( fn get_app_cache_path() -> Result { let cache_dir_candidates = [ - env::var_os("XDG_CACHE_HOME").map(PathBuf::from), + ensure_path_exists( + env::var_os("XDG_CACHE_HOME").map(PathBuf::from), + ), dirs::cache_dir(), ];