Skip to content

Commit

Permalink
cli: migrate remainder of Config::get() to StackedConfig::get()
Browse files Browse the repository at this point in the history
  • Loading branch information
@yuja
yuja committed Dec 4, 2024
1 parent 32e1a3e commit 2ff8750
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 111 deletions.
37 changes: 17 additions & 20 deletions cli/src/cli_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3078,7 +3078,10 @@ impl ValueParserFactory for RevisionArg {
}
}

fn get_string_or_array(config: &config::Config, key: &str) -> Result<Vec<String>, ConfigError> {
fn get_string_or_array(
config: &StackedConfig,
key: &'static str,
) -> Result<Vec<String>, ConfigError> {
config
.get(key)
.map(|string| vec![string])
Expand All @@ -3087,7 +3090,7 @@ fn get_string_or_array(config: &config::Config, key: &str) -> Result<Vec<String>

fn resolve_default_command(
ui: &Ui,
config: &config::Config,
config: &StackedConfig,
app: &Command,
mut string_args: Vec<String>,
) -> Result<Vec<String>, CommandError> {
Expand Down Expand Up @@ -3130,7 +3133,7 @@ fn resolve_default_command(

fn resolve_aliases(
ui: &Ui,
config: &config::Config,
config: &StackedConfig,
app: &Command,
mut string_args: Vec<String>,
) -> Result<Vec<String>, CommandError> {
Expand Down Expand Up @@ -3240,15 +3243,15 @@ fn handle_early_args(
}
if !args.config_toml.is_empty() {
config.add_layer(parse_config_args(&args.config_toml)?);
ui.reset(&config.merge())?;
ui.reset(config)?;
}
Ok(())
}

fn handle_shell_completion(
ui: &Ui,
app: &Command,
config: &config::Config,
config: &StackedConfig,
cwd: &Path,
) -> Result<(), CommandError> {
let mut args = vec![];
Expand Down Expand Up @@ -3296,7 +3299,7 @@ pub fn expand_args(
ui: &Ui,
app: &Command,
args_os: impl IntoIterator<Item = OsString>,
config: &config::Config,
config: &StackedConfig,
) -> Result<Vec<String>, CommandError> {
let mut string_args: Vec<String> = vec![];
for arg_os in args_os {
Expand Down Expand Up @@ -3509,11 +3512,7 @@ impl CliRunner {
}

#[instrument(skip_all)]
fn run_internal(
self,
ui: &mut Ui,
mut stacked_config: StackedConfig,
) -> Result<(), CommandError> {
fn run_internal(self, ui: &mut Ui, mut config: StackedConfig) -> Result<(), CommandError> {
// `cwd` is canonicalized for consistency with `Workspace::workspace_root()` and
// to easily compute relative paths between them.
let cwd = env::current_dir()
Expand All @@ -3532,12 +3531,11 @@ impl CliRunner {
.workspace_loader_factory
.create(find_workspace_dir(&cwd))
.map_err(|err| map_workspace_load_error(err, None));
config_env.reload_user_config(&mut stacked_config)?;
config_env.reload_user_config(&mut config)?;
if let Ok(loader) = &maybe_cwd_workspace_loader {
config_env.reset_repo_path(loader.repo_path());
config_env.reload_repo_config(&mut stacked_config)?;
config_env.reload_repo_config(&mut config)?;
}
let config = stacked_config.merge();
ui.reset(&config).map_err(|e| {
let user_config_path = config_env.existing_user_config_path();
let repo_config_path = config_env.existing_repo_config_path();
Expand All @@ -3559,9 +3557,9 @@ impl CliRunner {
&self.app,
&self.tracing_subscription,
&string_args,
&mut stacked_config,
&mut config,
)
.map_err(|err| map_clap_cli_error(err, ui, &stacked_config))?;
.map_err(|err| map_clap_cli_error(err, ui, &config))?;
for process_global_args_fn in self.process_global_args_fns {
process_global_args_fn(ui, &matches)?;
}
Expand All @@ -3573,14 +3571,13 @@ impl CliRunner {
.create(&cwd.join(path))
.map_err(|err| map_workspace_load_error(err, Some(path)))?;
config_env.reset_repo_path(loader.repo_path());
config_env.reload_repo_config(&mut stacked_config)?;
config_env.reload_repo_config(&mut config)?;
Ok(loader)
} else {
maybe_cwd_workspace_loader
};

// Apply workspace configs and --config-toml arguments.
let config = stacked_config.merge();
ui.reset(&config)?;

// If -R is specified, check if the expanded arguments differ. Aliases
Expand All @@ -3595,7 +3592,7 @@ impl CliRunner {
}
}

let settings = UserSettings::from_config(stacked_config);
let settings = UserSettings::from_config(config);
let command_helper_data = CommandHelperData {
app: self.app,
cwd,
Expand Down Expand Up @@ -3631,7 +3628,7 @@ impl CliRunner {
.build()
.unwrap();
let stacked_config = config_from_environment(config);
let mut ui = Ui::with_config(&stacked_config.merge())
let mut ui = Ui::with_config(&stacked_config)
.expect("default config should be valid, env vars are stringly typed");
let result = self.run_internal(&mut ui, stacked_config);
let exit_code = handle_command_result(&mut ui, result);
Expand Down
13 changes: 6 additions & 7 deletions cli/src/complete.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -676,19 +676,18 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> {
// child process. This shouldn't fail, since none of the global args are
// required.
let app = crate::commands::default_app();
let mut stacked_config = config_from_environment(default_config());
let ui = Ui::with_config(&stacked_config.merge()).expect("default config should be valid");
let mut config = config_from_environment(default_config());
let ui = Ui::with_config(&config).expect("default config should be valid");
let cwd = std::env::current_dir()
.and_then(|cwd| cwd.canonicalize())
.map_err(user_error)?;
let mut config_env = ConfigEnv::from_environment()?;
let maybe_cwd_workspace_loader = DefaultWorkspaceLoaderFactory.create(find_workspace_dir(&cwd));
let _ = config_env.reload_user_config(&mut stacked_config);
let _ = config_env.reload_user_config(&mut config);
if let Ok(loader) = &maybe_cwd_workspace_loader {
config_env.reset_repo_path(loader.repo_path());
let _ = config_env.reload_repo_config(&mut stacked_config);
let _ = config_env.reload_repo_config(&mut config);
}
let config = stacked_config.merge();
// skip 2 because of the clap_complete prelude: jj -- jj <actual args...>
let args = std::env::args_os().skip(2);
let args = expand_args(&ui, &app, args, &config)?;
Expand All @@ -704,7 +703,7 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> {
// Try to update repo-specific config on a best-effort basis.
if let Ok(loader) = DefaultWorkspaceLoaderFactory.create(&cwd.join(&repository)) {
config_env.reset_repo_path(loader.repo_path());
let _ = config_env.reload_repo_config(&mut stacked_config);
let _ = config_env.reload_repo_config(&mut config);
}
cmd_args.push("--repository".into());
cmd_args.push(repository);
Expand Down Expand Up @@ -745,7 +744,7 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> {
cmd: current_exe,
args: cmd_args,
};
let settings = UserSettings::from_config(stacked_config);
let settings = UserSettings::from_config(config);

Ok((builder, settings))
}
Expand Down
33 changes: 16 additions & 17 deletions cli/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -687,29 +687,28 @@ impl TryFrom<Vec<String>> for NonEmptyCommandArgsVec {
mod tests {
use anyhow::anyhow;
use assert_matches::assert_matches;
use indoc::indoc;
use maplit::hashmap;

use super::*;

#[test]
fn test_command_args() {
let config = config::Config::builder()
.set_override("empty_array", Vec::<String>::new())
.unwrap()
.set_override("empty_string", "")
.unwrap()
.set_override("array", vec!["emacs", "-nw"])
.unwrap()
.set_override("string", "emacs -nw")
.unwrap()
.set_override("structured.env.KEY1", "value1")
.unwrap()
.set_override("structured.env.KEY2", "value2")
.unwrap()
.set_override("structured.command", vec!["emacs", "-nw"])
.unwrap()
.build()
.unwrap();
let mut config = StackedConfig::empty();
config.add_layer(
ConfigLayer::parse(
ConfigSource::User,
indoc! {"
empty_array = []
empty_string = ''
array = ['emacs', '-nw']
string = 'emacs -nw'
structured.env = { KEY1 = 'value1', KEY2 = 'value2' }
structured.command = ['emacs', '-nw']
"},
)
.unwrap(),
);

assert!(config.get::<CommandNameAndArgs>("empty_array").is_err());

Expand Down
Loading

0 comments on commit 2ff8750

Please sign in to comment.