From 36e83ed26011b2ac544da8312980f71fb8288b59 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 28 Nov 2024 23:12:32 +0900 Subject: [PATCH] cli: move get_new_config_file_path() to commands::config There aren't any other callers internally, and it is a thin wrapper around ConfigEnv functions which can be easily implemented. --- cli/src/cli_util.rs | 23 ----------------------- cli/src/commands/config/edit.rs | 3 +-- cli/src/commands/config/mod.rs | 27 +++++++++++++++++++++++---- cli/src/commands/config/path.rs | 3 +-- cli/src/commands/config/set.rs | 3 +-- cli/src/commands/config/unset.rs | 3 +-- 6 files changed, 27 insertions(+), 35 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 91fb6ed49f..4fc1963b69 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -58,7 +58,6 @@ use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; use jj_lib::config::ConfigError; -use jj_lib::config::ConfigSource; use jj_lib::config::StackedConfig; use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::file_util; @@ -2760,28 +2759,6 @@ impl LogContentFormat { } } -pub fn get_new_config_file_path( - config_source: ConfigSource, - command: &CommandHelper, -) -> Result<&Path, CommandError> { - let config_env = command.config_env(); - let edit_path = match config_source { - // TODO(#531): Special-case for editors that can't handle viewing directories? - ConfigSource::User => config_env - .new_user_config_path()? - .ok_or_else(|| user_error("No user config path found to edit"))?, - ConfigSource::Repo => config_env - .new_repo_config_path() - .ok_or_else(|| user_error("No repo config path found to edit"))?, - _ => { - return Err(user_error(format!( - "Can't get path for config source {config_source:?}" - ))); - } - }; - Ok(edit_path) -} - pub fn run_ui_editor(settings: &UserSettings, edit_path: &Path) -> Result<(), CommandError> { // Work around UNC paths not being well supported on Windows (no-op for // non-Windows): https://github.com/martinvonz/jj/issues/3986 diff --git a/cli/src/commands/config/edit.rs b/cli/src/commands/config/edit.rs index 33ae6bedea..de3269c4c1 100644 --- a/cli/src/commands/config/edit.rs +++ b/cli/src/commands/config/edit.rs @@ -15,7 +15,6 @@ use tracing::instrument; use super::ConfigLevelArgs; -use crate::cli_util::get_new_config_file_path; use crate::cli_util::run_ui_editor; use crate::cli_util::CommandHelper; use crate::command_error::CommandError; @@ -37,6 +36,6 @@ pub fn cmd_config_edit( command: &CommandHelper, args: &ConfigEditArgs, ) -> Result<(), CommandError> { - let config_path = get_new_config_file_path(args.level.expect_source_kind(), command)?; + let config_path = args.level.new_config_file_path(command.config_env())?; run_ui_editor(command.settings(), config_path) } diff --git a/cli/src/commands/config/mod.rs b/cli/src/commands/config/mod.rs index 189a91bfdf..6f85f532b5 100644 --- a/cli/src/commands/config/mod.rs +++ b/cli/src/commands/config/mod.rs @@ -19,6 +19,8 @@ mod path; mod set; mod unset; +use std::path::Path; + use jj_lib::config::ConfigSource; use tracing::instrument; @@ -35,7 +37,9 @@ use self::set::ConfigSetArgs; use self::unset::cmd_config_unset; use self::unset::ConfigUnsetArgs; use crate::cli_util::CommandHelper; +use crate::command_error::user_error; use crate::command_error::CommandError; +use crate::config::ConfigEnv; use crate::ui::Ui; #[derive(clap::Args, Clone, Debug)] @@ -51,10 +55,6 @@ pub(crate) struct ConfigLevelArgs { } impl ConfigLevelArgs { - fn expect_source_kind(&self) -> ConfigSource { - self.get_source_kind().expect("No config_level provided") - } - fn get_source_kind(&self) -> Option { if self.user { Some(ConfigSource::User) @@ -64,6 +64,25 @@ impl ConfigLevelArgs { None } } + + fn new_config_file_path<'a>( + &self, + config_env: &'a ConfigEnv, + ) -> Result<&'a Path, CommandError> { + if self.user { + // TODO(#531): Special-case for editors that can't handle viewing + // directories? + config_env + .new_user_config_path()? + .ok_or_else(|| user_error("No user config path found to edit")) + } else if self.repo { + config_env + .new_repo_config_path() + .ok_or_else(|| user_error("No repo config path found to edit")) + } else { + panic!("No config_level provided") + } + } } /// Manage config options diff --git a/cli/src/commands/config/path.rs b/cli/src/commands/config/path.rs index d1c9d3a887..b157495bed 100644 --- a/cli/src/commands/config/path.rs +++ b/cli/src/commands/config/path.rs @@ -17,7 +17,6 @@ use std::io::Write as _; use tracing::instrument; use super::ConfigLevelArgs; -use crate::cli_util::get_new_config_file_path; use crate::cli_util::CommandHelper; use crate::command_error::user_error; use crate::command_error::CommandError; @@ -40,7 +39,7 @@ pub fn cmd_config_path( command: &CommandHelper, args: &ConfigPathArgs, ) -> Result<(), CommandError> { - let config_path = get_new_config_file_path(args.level.expect_source_kind(), command)?; + let config_path = args.level.new_config_file_path(command.config_env())?; writeln!( ui.stdout(), "{}", diff --git a/cli/src/commands/config/set.rs b/cli/src/commands/config/set.rs index 342c502f3f..2ddf032f0a 100644 --- a/cli/src/commands/config/set.rs +++ b/cli/src/commands/config/set.rs @@ -21,7 +21,6 @@ use jj_lib::repo::Repo; use tracing::instrument; use super::ConfigLevelArgs; -use crate::cli_util::get_new_config_file_path; use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandHelper; use crate::command_error::user_error; @@ -54,7 +53,7 @@ pub fn cmd_config_set( command: &CommandHelper, args: &ConfigSetArgs, ) -> Result<(), CommandError> { - let config_path = get_new_config_file_path(args.level.expect_source_kind(), command)?; + let config_path = args.level.new_config_file_path(command.config_env())?; if config_path.is_dir() { return Err(user_error(format!( "Can't set config in path {path} (dirs not supported)", diff --git a/cli/src/commands/config/unset.rs b/cli/src/commands/config/unset.rs index befd276071..6f96028df1 100644 --- a/cli/src/commands/config/unset.rs +++ b/cli/src/commands/config/unset.rs @@ -17,7 +17,6 @@ use jj_lib::config::ConfigNamePathBuf; use tracing::instrument; use super::ConfigLevelArgs; -use crate::cli_util::get_new_config_file_path; use crate::cli_util::CommandHelper; use crate::command_error::user_error; use crate::command_error::CommandError; @@ -40,7 +39,7 @@ pub fn cmd_config_unset( command: &CommandHelper, args: &ConfigUnsetArgs, ) -> Result<(), CommandError> { - let config_path = get_new_config_file_path(args.level.expect_source_kind(), command)?; + let config_path = args.level.new_config_file_path(command.config_env())?; if config_path.is_dir() { return Err(user_error(format!( "Can't set config in path {path} (dirs not supported)",