From 12465193a4f6da8334e8f50acd77b33c41f265dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joaqu=C3=ADn=20Tri=C3=B1anes?= Date: Fri, 13 Jun 2025 01:23:38 +0200 Subject: [PATCH] cli: Use latest specified flag value when repeated (#15919) # Description This PR makes the last specified CLI arguments take precedence over the earlier ones. Existing command line tools that align with the new behaviour include: - `neovim`: `nvim -u a.lua -u b.lua` will use `b.lua` - `ripgrep`: you can have `--smart-case` in your user config but override it later with `--case-sensitive` or `--ignore-case` (not exactly the same flag override as the one I'm talking about but I think it's still a valid example of latter flags taking precedence over the first ones) I think a flag defined last can be considered an override. This allows having a `nu` alias that includes some default config (`alias nu="nu --config something.nu"`) but being able to override that default config as if using `nu` normally. ## Example ```sh nu --config config1.nu --config config2.nu -c '$nu.config-path' ``` The current behavior would print `config1.nu`, and the new one would print `config2.nu` ## Implementation Just `.rev()` the iterator to search for arguments starting from the end of the list. To support that I had to modify the return type of `named_iter` (I couldn't find a more generic way than `DoubleEndedIterator`). # User-Facing Changes - Users passing repeated flags and relying in nushell using the first value will experience breakage. Given that right now there's no point in passing a flag multiple times I guess not many users will be affected # Tests + Formatting I added a test that checks the new behavior with `--config` and `--env-config`. I'm happy to add more cases if needed # After Submitting --- crates/nu-protocol/src/ast/call.rs | 7 ++++--- tests/repl/test_config_path.rs | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/nu-protocol/src/ast/call.rs b/crates/nu-protocol/src/ast/call.rs index 5407ee5fa4..868ec53bdb 100644 --- a/crates/nu-protocol/src/ast/call.rs +++ b/crates/nu-protocol/src/ast/call.rs @@ -128,7 +128,8 @@ impl Call { pub fn named_iter( &self, - ) -> impl Iterator, Option>, Option)> { + ) -> impl DoubleEndedIterator, Option>, Option)> + { self.arguments.iter().filter_map(|arg| match arg { Argument::Named(named) => Some(named), Argument::Positional(_) => None, @@ -222,7 +223,7 @@ impl Call { } pub fn get_flag_expr(&self, flag_name: &str) -> Option<&Expression> { - for name in self.named_iter() { + for name in self.named_iter().rev() { if flag_name == name.0.item { return name.2.as_ref(); } @@ -232,7 +233,7 @@ impl Call { } pub fn get_named_arg(&self, flag_name: &str) -> Option> { - for name in self.named_iter() { + for name in self.named_iter().rev() { if flag_name == name.0.item { return Some(name.0.clone()); } diff --git a/tests/repl/test_config_path.rs b/tests/repl/test_config_path.rs index 8ed90f913a..5b78ed41dd 100644 --- a/tests/repl/test_config_path.rs +++ b/tests/repl/test_config_path.rs @@ -242,6 +242,29 @@ fn test_alternate_config_path() { assert_eq!(actual.out, env_path.to_string_lossy().to_string()); } +#[test] +fn use_last_config_path() { + let config_file = "crates/nu-utils/src/default_files/scaffold_config.nu"; + let env_file = "crates/nu-utils/src/default_files/scaffold_env.nu"; + + let cwd = std::env::current_dir().expect("Could not get current working directory"); + + let config_path = + nu_path::canonicalize_with(config_file, &cwd).expect("Could not get config path"); + let actual = nu!( + cwd: &cwd, + format!("nu --config non-existing-path --config another-random-path.nu --config {config_path:?} -c '$nu.config-path'") + ); + assert_eq!(actual.out, config_path.to_string_lossy().to_string()); + + let env_path = nu_path::canonicalize_with(env_file, &cwd).expect("Could not get env path"); + let actual = nu!( + cwd: &cwd, + format!("nu --env-config non-existing-path --env-config {env_path:?} -c '$nu.env-path'") + ); + assert_eq!(actual.out, env_path.to_string_lossy().to_string()); +} + #[test] fn test_xdg_config_empty() { Playground::setup("xdg_config_empty", |_, playground| {