diff --git a/src/application.rs b/src/application.rs index 7791b7d0c..55841ff58 100644 --- a/src/application.rs +++ b/src/application.rs @@ -9,7 +9,7 @@ pub(crate) use crate::application::app_data::AppData; use crate::{ Args, Exit, - config::{Config, ConfigLoader, DiffIgnoreWhitespaceSetting}, + config::{Config, ConfigError, ConfigErrorCause, ConfigLoader, DiffIgnoreWhitespaceSetting}, diff::{self, CommitDiffLoader, CommitDiffLoaderOptions}, display::Display, git::open_repository_from_env, @@ -34,15 +34,16 @@ where ModuleProvider: module::ModuleProvider + Send + 'static impl Application where ModuleProvider: module::ModuleProvider + Send + 'static { - pub(crate) fn new(args: &Args, event_provider: EventProvider, tui: Tui) -> Result + pub(crate) fn new(args: Args, git_config_parameters: Vec<(String, String)>, event_provider: EventProvider, tui: Tui) -> Result where EventProvider: EventReaderFn, Tui: crate::display::Tui + Send + 'static, { let filepath = Self::filepath_from_args(args)?; let repository = Self::open_repository()?; - let config_loader = ConfigLoader::from(repository); - let config = Self::load_config(&config_loader)?; + let config_loader = ConfigLoader::with_overrides(repository, git_config_parameters); + let config = Self::load_config(&config_loader) + .map_err(|err| Exit::new(ExitStatus::ConfigError, format!("{err:#}").as_str()))?; let todo_file = Arc::new(Mutex::new(Self::load_todo_file(filepath.as_str(), &config)?)); let display = Display::new(tui, &config.theme); @@ -144,7 +145,7 @@ where ModuleProvider: module::ModuleProvider + Send + 'static Ok(()) } - fn filepath_from_args(args: &Args) -> Result { + fn filepath_from_args(args: Args) -> Result { args.todo_file_path().map(String::from).ok_or_else(|| { Exit::new( ExitStatus::StateError, @@ -162,8 +163,11 @@ where ModuleProvider: module::ModuleProvider + Send + 'static }) } - fn load_config(config_loader: &ConfigLoader) -> Result { - Config::try_from(config_loader).map_err(|err| Exit::new(ExitStatus::ConfigError, format!("{err:#}").as_str())) + fn load_config(config_loader: &ConfigLoader) -> Result { + let config = config_loader + .load_config() + .map_err(|e| ConfigError::new_read_error("", ConfigErrorCause::GitError(e)))?; + Config::new_with_config(Some(&config)) } fn todo_file_options(config: &Config) -> TodoFileOptions { @@ -208,8 +212,6 @@ where ModuleProvider: module::ModuleProvider + Send + 'static #[cfg(all(unix, test))] mod tests { - use std::ffi::OsString; - use claims::assert_ok; use super::*; @@ -219,19 +221,10 @@ mod tests { module::Modules, runtime::{Installer, RuntimeError}, test_helpers::{ - DefaultTestModule, - TestModuleProvider, - create_config, - create_event_reader, - mocks, - with_git_directory, + DefaultTestModule, TestModuleProvider, create_config, create_event_reader, mocks, with_git_directory, }, }; - fn args(args: &[&str]) -> Args { - Args::try_from(args.iter().map(OsString::from).collect::>()).unwrap() - } - fn create_mocked_crossterm() -> mocks::CrossTerm { let mut crossterm = mocks::CrossTerm::new(); crossterm.set_size(Size::new(300, 120)); @@ -242,8 +235,7 @@ mod tests { ($app:expr) => { if let Err(e) = $app { e - } - else { + } else { panic!("Application is not in an error state"); } }; @@ -253,8 +245,12 @@ mod tests { #[serial_test::serial] fn load_filepath_from_args_failure() { let event_provider = create_event_reader(|| Ok(None)); - let application: Result>, Exit> = - Application::new(&args(&[]), event_provider, create_mocked_crossterm()); + let application: Result>, Exit> = Application::new( + Args::from_os_strings(Vec::new()).unwrap(), + Vec::new(), + event_provider, + create_mocked_crossterm(), + ); let exit = application_error!(application); assert_eq!(exit.get_status(), &ExitStatus::StateError); assert!( @@ -268,8 +264,12 @@ mod tests { fn load_repository_failure() { with_git_directory("fixtures/not-a-repository", |_| { let event_provider = create_event_reader(|| Ok(None)); - let application: Result>, Exit> = - Application::new(&args(&["todofile"]), event_provider, create_mocked_crossterm()); + let application: Result>, Exit> = Application::new( + Args::from_strs(["todofile"]).unwrap(), + Vec::new(), + event_provider, + create_mocked_crossterm(), + ); let exit = application_error!(application); assert_eq!(exit.get_status(), &ExitStatus::StateError); assert!(exit.get_message().unwrap().contains("Unable to load Git repository: ")); @@ -280,8 +280,12 @@ mod tests { fn load_config_failure() { with_git_directory("fixtures/invalid-config", |_| { let event_provider = create_event_reader(|| Ok(None)); - let application: Result>, Exit> = - Application::new(&args(&["rebase-todo"]), event_provider, create_mocked_crossterm()); + let application: Result>, Exit> = Application::new( + Args::from_strs(["rebase-todo"]).unwrap(), + Vec::new(), + event_provider, + create_mocked_crossterm(), + ); let exit = application_error!(application); assert_eq!(exit.get_status(), &ExitStatus::ConfigError); }); @@ -321,8 +325,12 @@ mod tests { fn load_todo_file_load_error() { with_git_directory("fixtures/simple", |_| { let event_provider = create_event_reader(|| Ok(None)); - let application: Result>, Exit> = - Application::new(&args(&["does-not-exist"]), event_provider, create_mocked_crossterm()); + let application: Result>, Exit> = Application::new( + Args::from_strs(["does-not-exist"]).unwrap(), + Vec::new(), + event_provider, + create_mocked_crossterm(), + ); let exit = application_error!(application); assert_eq!(exit.get_status(), &ExitStatus::FileReadError); }); @@ -334,7 +342,8 @@ mod tests { let rebase_todo = format!("{git_dir}/rebase-todo-noop"); let event_provider = create_event_reader(|| Ok(None)); let application: Result>, Exit> = Application::new( - &args(&[rebase_todo.as_str()]), + Args::from_strs([rebase_todo]).unwrap(), + Vec::new(), event_provider, create_mocked_crossterm(), ); @@ -349,7 +358,8 @@ mod tests { let rebase_todo = format!("{git_dir}/rebase-todo-empty"); let event_provider = create_event_reader(|| Ok(None)); let application: Result>, Exit> = Application::new( - &args(&[rebase_todo.as_str()]), + Args::from_strs([rebase_todo]).unwrap(), + Vec::new(), event_provider, create_mocked_crossterm(), ); @@ -382,7 +392,8 @@ mod tests { let rebase_todo = format!("{git_dir}/rebase-todo"); let event_provider = create_event_reader(|| Ok(Some(Event::Key(KeyEvent::from(KeyCode::Char('W')))))); let mut application: Application = Application::new( - &args(&[rebase_todo.as_str()]), + Args::from_strs([rebase_todo]).unwrap(), + Vec::new(), event_provider, create_mocked_crossterm(), ) @@ -408,7 +419,8 @@ mod tests { let rebase_todo = format!("{git_dir}/rebase-todo"); let event_provider = create_event_reader(|| Ok(Some(Event::Key(KeyEvent::from(KeyCode::Char('W')))))); let mut application: Application = Application::new( - &args(&[rebase_todo.as_str()]), + Args::from_strs([rebase_todo]).unwrap(), + Vec::new(), event_provider, create_mocked_crossterm(), ) @@ -433,7 +445,8 @@ mod tests { )))) }); let mut application: Application = Application::new( - &args(&[rebase_todo.as_str()]), + Args::from_strs([rebase_todo]).unwrap(), + Vec::new(), event_provider, create_mocked_crossterm(), ) @@ -449,7 +462,8 @@ mod tests { let rebase_todo = format!("{git_dir}/rebase-todo"); let event_provider = create_event_reader(|| Ok(Some(Event::Key(KeyEvent::from(KeyCode::Char('W')))))); let mut application: Application = Application::new( - &args(&[rebase_todo.as_str()]), + Args::from_strs([rebase_todo]).unwrap(), + Vec::new(), event_provider, create_mocked_crossterm(), ) diff --git a/src/arguments.rs b/src/arguments.rs index cf847496d..be73f360f 100644 --- a/src/arguments.rs +++ b/src/arguments.rs @@ -26,12 +26,14 @@ impl Args { pub(crate) fn todo_file_path(&self) -> Option<&str> { self.todo_file_path.as_deref() } -} -impl TryFrom> for Args { - type Error = Exit; + #[cfg(test)] + pub(crate) fn from_strs(args: impl IntoIterator>) -> Result { + let args = args.into_iter().map(|it| OsString::from(it.as_ref())).collect(); + Self::from_os_strings(args) + } - fn try_from(args: Vec) -> Result { + pub(crate) fn from_os_strings(args: Vec) -> Result { let mut pargs = Arguments::from_vec(args); let mode = if pargs.contains(["-h", "--help"]) { @@ -59,21 +61,17 @@ impl TryFrom> for Args { mod tests { use super::*; - fn create_args(args: &[&str]) -> Vec { - args.iter().map(OsString::from).collect() - } - #[test] fn mode_help() { - assert_eq!(Args::try_from(create_args(&["-h"])).unwrap().mode(), &Mode::Help); - assert_eq!(Args::try_from(create_args(&["--help"])).unwrap().mode(), &Mode::Help); + assert_eq!(Args::from_strs(["-h"]).unwrap().mode(), &Mode::Help); + assert_eq!(Args::from_strs(["--help"]).unwrap().mode(), &Mode::Help); } #[test] fn mode_version() { - assert_eq!(Args::try_from(create_args(&["-v"])).unwrap().mode(), &Mode::Version); + assert_eq!(Args::from_strs(["-v"]).unwrap().mode(), &Mode::Version); assert_eq!( - Args::try_from(create_args(&["--version"])).unwrap().mode(), + Args::from_strs(["--version"]).unwrap().mode(), &Mode::Version ); } @@ -81,21 +79,21 @@ mod tests { #[test] fn mode_license() { assert_eq!( - Args::try_from(create_args(&["--license"])).unwrap().mode(), + Args::from_strs(["--license"]).unwrap().mode(), &Mode::License ); } #[test] fn todo_file_ok() { - let args = Args::try_from(create_args(&["todofile"])).unwrap(); + let args = Args::from_strs(["todofile"]).unwrap(); assert_eq!(args.mode(), &Mode::Editor); assert_eq!(args.todo_file_path(), Some("todofile")); } #[test] fn todo_file_missing() { - let args = Args::try_from(create_args(&[])).unwrap(); + let args = Args::from_os_strings(Vec::new()).unwrap(); assert_eq!(args.mode(), &Mode::Editor); assert!(args.todo_file_path().is_none()); } @@ -105,6 +103,6 @@ mod tests { #[expect(unsafe_code)] fn todo_file_invalid() { let args = unsafe { vec![OsString::from(String::from_utf8_unchecked(vec![0xC3, 0x28]))] }; - _ = Args::try_from(args).unwrap_err(); + _ = Args::from_os_strings(args).unwrap_err(); } } diff --git a/src/config.rs b/src/config.rs index 327485f90..96e6dfa4e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -23,14 +23,12 @@ pub(crate) use self::{ config_loader::ConfigLoader, diff_ignore_whitespace_setting::DiffIgnoreWhitespaceSetting, diff_show_whitespace_setting::DiffShowWhitespaceSetting, + errors::{ConfigError, ConfigErrorCause, InvalidColorError}, git_config::GitConfig, key_bindings::KeyBindings, theme::Theme, }; -use crate::config::{ - errors::{ConfigError, ConfigErrorCause, InvalidColorError}, - utils::get_optional_string, -}; +use crate::config::utils::get_optional_string; const DEFAULT_SPACE_SYMBOL: &str = "\u{b7}"; // · const DEFAULT_TAB_SYMBOL: &str = "\u{2192}"; // → @@ -94,22 +92,6 @@ impl Config { } } -impl TryFrom<&ConfigLoader> for Config { - type Error = ConfigError; - - /// Creates a new Config instance loading the Git Config. - /// - /// # Errors - /// - /// Will return an `Err` if there is a problem loading the configuration. - fn try_from(config_loader: &ConfigLoader) -> Result { - let config = config_loader - .load_config() - .map_err(|e| ConfigError::new_read_error("", ConfigErrorCause::GitError(e)))?; - Self::new_with_config(Some(&config)) - } -} - impl TryFrom<&crate::git::Config> for Config { type Error = ConfigError; @@ -131,8 +113,9 @@ mod tests { #[test] fn try_from_config_loader() { with_temp_bare_repository(|repository| { - let loader = ConfigLoader::from(repository); - assert_ok!(Config::try_from(&loader)); + let loader = ConfigLoader::new(repository); + let config = assert_ok!(loader.load_config()); + assert_ok!(Config::try_from(&config)); }); } diff --git a/src/config/config_loader.rs b/src/config/config_loader.rs index e9c9bbc71..3dbbadb78 100644 --- a/src/config/config_loader.rs +++ b/src/config/config_loader.rs @@ -6,15 +6,37 @@ use crate::git::{Config, GitError}; pub(crate) struct ConfigLoader { repository: Repository, + overrides: Vec<(String, String)>, } impl ConfigLoader { - /// Load the git configuration for the repository. + #[cfg(test)] + pub(crate) fn new(repository: Repository) -> Self { + let overrides = Vec::new(); + Self { repository, overrides } + } + + pub(crate) fn with_overrides(repository: Repository, overrides: Vec<(String, String)>) -> Self { + Self { repository, overrides } + } + + /// Load the git configuration for the repository, + /// with any overrides taking priority over the values defined in the repositroy /// /// # Errors /// Will result in an error if the configuration is invalid. pub(crate) fn load_config(&self) -> Result { - self.repository.config().map_err(|e| GitError::ConfigLoad { cause: e }) + let into_git_error = |cause| GitError::ConfigLoad { cause }; + + let mut config = self.repository.config().map_err(into_git_error)?; + for (name, value) in &self.overrides { + if value.is_empty() { + config.set_bool(name, true).map_err(into_git_error)?; + } else { + config.set_str(name, value).map_err(into_git_error)?; + } + } + Ok(config) } pub(crate) fn eject_repository(self) -> Repository { @@ -22,12 +44,6 @@ impl ConfigLoader { } } -impl From for ConfigLoader { - fn from(repository: Repository) -> Self { - Self { repository } - } -} - impl Debug for ConfigLoader { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { f.debug_struct("ConfigLoader") @@ -49,7 +65,7 @@ mod unix_tests { #[test] fn load_config() { with_temp_bare_repository(|repository| { - let config = ConfigLoader::from(repository); + let config = ConfigLoader::new(repository); assert_ok!(config.load_config()); }); } @@ -58,7 +74,7 @@ mod unix_tests { fn fmt() { with_temp_repository(|repository| { let path = repository.path().canonicalize().unwrap(); - let loader = ConfigLoader::from(repository); + let loader = ConfigLoader::new(repository); let formatted = format!("{loader:?}"); assert_eq!( formatted, diff --git a/src/editor.rs b/src/editor.rs index de87efcfa..704d400dd 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -11,8 +11,8 @@ use crate::{ }; #[cfg(not(tarpaulin_include))] -pub(crate) fn run(args: &Args) -> Exit { - let mut application: Application = match Application::new(args, read_event, CrossTerm::new()) { +pub(crate) fn run(args: Args, git_config_parameters: Vec<(String, String)>) -> Exit { + let mut application: Application = match Application::new(args, git_config_parameters, read_event, CrossTerm::new()) { Ok(app) => app, Err(exit) => return exit, }; @@ -25,21 +25,19 @@ pub(crate) fn run(args: &Args) -> Exit { #[cfg(test)] mod tests { - use std::{ffi::OsString, path::Path}; + use std::path::Path; use super::*; use crate::test_helpers::with_git_directory; - fn args(args: &[&str]) -> Args { - Args::try_from(args.iter().map(OsString::from).collect::>()).unwrap() - } - #[test] fn successful_run() { with_git_directory("fixtures/simple", |path| { - let todo_file = Path::new(path).join("rebase-todo-empty"); + let todo_file = Path::new(path).join("rebase-todo-empty").into_os_string(); + let args = Args::from_os_strings(vec![todo_file]).unwrap(); + let git_config_parameters = Vec::new(); assert_eq!( - run(&args(&[todo_file.to_str().unwrap()])).get_status(), + run(args, git_config_parameters).get_status(), &ExitStatus::Good ); }); @@ -48,9 +46,11 @@ mod tests { #[test] fn error_on_application_create() { with_git_directory("fixtures/simple", |path| { - let todo_file = Path::new(path).join("does-not-exist"); + let todo_file = Path::new(path).join("does-not-exist").into_os_string(); + let args = Args::from_os_strings(vec![todo_file]).unwrap(); + let git_config_parameters = Vec::new(); assert_eq!( - run(&args(&[todo_file.to_str().unwrap()])).get_status(), + run(args, git_config_parameters).get_status(), &ExitStatus::FileReadError ); }); diff --git a/src/main.rs b/src/main.rs index 1a0d861b2..68dd03ee9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -72,24 +72,65 @@ use crate::{ }; #[must_use] -fn run(os_args: Vec) -> Exit { - match Args::try_from(os_args) { +fn run(os_args: Vec, git_config_parameters: Vec<(String, String)>) -> Exit { + match Args::from_os_strings(os_args) { Err(err) => err, Ok(args) => { match *args.mode() { Mode::Help => help::run(), Mode::Version => version::run(), Mode::License => license::run(), - Mode::Editor => editor::run(&args), + Mode::Editor => editor::run(args, git_config_parameters), } }, } } +/// returns a pair of name/value pairs passed as overrides to `git`. +/// +/// input here is expected to come from `git -c`, and is in the form +/// of `-c =` pairs, for example `-c interactive-rebase-tool.diffTabWidth=4`. +/// separated by a single whitespace +/// +/// notes: +/// 1. the key/value have both gone through git's shell quoting. +/// from `gix-quote`: "every single-quote `'` is escaped as `'\''`, +/// every exclamation mark `!` is escaped as `'\!'`, and the entire string +/// is enclosed in single quotes." +/// 2. if input doesn't contain a '=', a `=` is appended between +/// `key` and `value`. `value`, will be an empty string. +/// 3. if input does contain a '=' but `value` is empty, `value` +/// will also be an empty string. +/// 4. an empty string will later be interpreted as `true`, +/// but we don't do this here. +pub(crate) fn parse_git_config_parameters(env_var: OsString) -> Vec<(String, String)> { + // we expect valid UTF-8 from the shell/git, so we don't handle errors here. + let Some(env_var) = env_var.to_str() else { + return Vec::new(); + }; + + // naive implementation: assumes correctly-escaped strings, efficiency isn't a priority + fn unescape(s: &str) -> String { + let s = s.trim_matches('\''); + let mut s = s.replace("\\'", "\'"); + s = s.replace("\\!", "!"); + s + } + + env_var + .split_ascii_whitespace() + .filter_map(|pair| pair.split_once('=')) + .map(|(name, value)| (unescape(name), unescape(value))) + .collect() +} #[expect(clippy::print_stderr, reason = "Required to print error message.")] #[cfg(not(tarpaulin_include))] fn main() -> impl Termination { - let exit = run(env::args_os().skip(1).collect()); + let args = env::args_os().skip(1).collect(); + let git_config_parameters = env::var_os("GIT_CONFIG_PARAMETERS") + .map(parse_git_config_parameters) + .unwrap_or_default(); + let exit = run(args, git_config_parameters); if let Some(message) = exit.get_message() { eprintln!("{message}"); } diff --git a/src/tests.rs b/src/tests.rs index 9e51aad72..db3181cc5 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -3,14 +3,12 @@ use std::path::Path; use super::*; use crate::{module::ExitStatus, test_helpers::with_git_directory}; -fn args(args: &[&str]) -> Vec { - args.iter().map(OsString::from).collect::>() -} - #[test] #[serial_test::serial] fn successful_run_help() { - let exit = run(args(&["--help"])); + let args = ["--help"].into_iter().map(OsString::from).collect(); + let git_config_parameters = Vec::new(); + let exit = run(args, git_config_parameters); assert!(exit.get_message().unwrap().contains("USAGE:")); assert_eq!(exit.get_status(), &ExitStatus::Good); } @@ -18,7 +16,9 @@ fn successful_run_help() { #[test] #[serial_test::serial] fn successful_run_version() { - let exit = run(args(&["--version"])); + let args = ["--version"].into_iter().map(OsString::from).collect(); + let git_config_parameters = Vec::new(); + let exit = run(args, git_config_parameters); assert!(exit.get_message().unwrap().starts_with("interactive-rebase-tool")); assert_eq!(exit.get_status(), &ExitStatus::Good); } @@ -26,7 +26,9 @@ fn successful_run_version() { #[test] #[serial_test::serial] fn successful_run_license() { - let exit = run(args(&["--license"])); + let args = ["--license"].into_iter().map(OsString::from).collect(); + let git_config_parameters = Vec::new(); + let exit = run(args, git_config_parameters); assert!( exit.get_message() .unwrap() @@ -38,9 +40,11 @@ fn successful_run_license() { #[test] fn successful_run_editor() { with_git_directory("fixtures/simple", |path| { - let todo_file = Path::new(path).join("rebase-todo-empty"); + let todo_file = Path::new(path).join("rebase-todo-empty").into_os_string(); + let args = vec![todo_file]; + let git_config_parameters = Vec::new(); assert_eq!( - run(args(&[todo_file.to_str().unwrap()])).get_status(), + run(args, git_config_parameters).get_status(), &ExitStatus::Good ); }); @@ -52,5 +56,6 @@ fn successful_run_editor() { #[expect(unsafe_code)] fn error() { let args = unsafe { vec![OsString::from(String::from_utf8_unchecked(vec![0xC3, 0x28]))] }; - assert_eq!(run(args).get_status(), &ExitStatus::StateError); + let git_config_parameters = Vec::new(); + assert_eq!(run(args, git_config_parameters).get_status(), &ExitStatus::StateError); }