From 6d26d53eab656c76831a6701bf591d3c77e2a3dd Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 21 Nov 2024 22:47:38 +0900 Subject: [PATCH] config: add type alias for config::ConfigError We'll probably add our own ConfigError type later. --- cli/src/cli_util.rs | 8 +++----- cli/src/command_error.rs | 5 +++-- cli/src/commands/config/get.rs | 3 ++- cli/src/commands/log.rs | 3 ++- cli/src/commands/operation/log.rs | 6 ++---- cli/src/config.rs | 23 ++++++++++++----------- cli/src/diff_util.rs | 24 ++++++++++++------------ cli/src/formatter.rs | 11 ++++++----- cli/src/graphlog.rs | 3 ++- cli/src/merge_tools/mod.rs | 4 ++-- cli/src/ui.rs | 3 ++- lib/src/config.rs | 11 ++++++----- lib/src/fsmonitor.rs | 2 +- lib/src/gpg_signing.rs | 3 ++- lib/src/settings.rs | 17 +++++++++-------- lib/src/signing.rs | 3 ++- lib/src/ssh_signing.rs | 3 ++- 17 files changed, 70 insertions(+), 62 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 7f76690a7..66d1462b8 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -56,6 +56,7 @@ use jj_lib::backend::CommitId; use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; +use jj_lib::config::ConfigError; use jj_lib::config::ConfigNamePathBuf; use jj_lib::file_util; use jj_lib::fileset; @@ -2685,7 +2686,7 @@ pub struct LogContentFormat { impl LogContentFormat { /// Creates new formatting helper for the terminal. - pub fn new(ui: &Ui, settings: &UserSettings) -> Result { + pub fn new(ui: &Ui, settings: &UserSettings) -> Result { Ok(LogContentFormat { width: ui.term_width(), word_wrap: settings.config().get_bool("ui.log-word-wrap")?, @@ -3065,10 +3066,7 @@ impl ValueParserFactory for RevisionArg { } } -fn get_string_or_array( - config: &config::Config, - key: &str, -) -> Result, config::ConfigError> { +fn get_string_or_array(config: &config::Config, key: &str) -> Result, ConfigError> { config .get(key) .map(|string| vec![string]) diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 5070895eb..48c167296 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -22,6 +22,7 @@ use std::sync::Arc; use itertools::Itertools as _; use jj_lib::backend::BackendError; +use jj_lib::config::ConfigError; use jj_lib::dsl_util::Diagnostics; use jj_lib::fileset::FilePatternParseError; use jj_lib::fileset::FilesetParseError; @@ -238,8 +239,8 @@ impl From for CommandError { } } -impl From for CommandError { - fn from(err: config::ConfigError) -> Self { +impl From for CommandError { + fn from(err: ConfigError) -> Self { config_error(err) } } diff --git a/cli/src/commands/config/get.rs b/cli/src/commands/config/get.rs index f5a97b351..7557cdacc 100644 --- a/cli/src/commands/config/get.rs +++ b/cli/src/commands/config/get.rs @@ -15,6 +15,7 @@ use std::io::Write as _; use clap_complete::ArgValueCandidates; +use jj_lib::config::ConfigError; use jj_lib::config::ConfigNamePathBuf; use tracing::instrument; @@ -51,7 +52,7 @@ pub fn cmd_config_get( .lookup_value(command.settings().config()) .and_then(|value| value.into_string()) .map_err(|err| match err { - config::ConfigError::Type { + ConfigError::Type { origin, unexpected, expected, diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index 299624316..bf55302cf 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -14,6 +14,7 @@ use clap_complete::ArgValueCandidates; use jj_lib::backend::CommitId; +use jj_lib::config::ConfigError; use jj_lib::graph::GraphEdgeType; use jj_lib::graph::ReverseGraphIterator; use jj_lib::graph::TopoGroupedGraphIterator; @@ -331,7 +332,7 @@ pub(crate) fn cmd_log( pub fn get_node_template( style: GraphStyle, settings: &UserSettings, -) -> Result { +) -> Result { let symbol = settings .config() .get_string("templates.log_node") diff --git a/cli/src/commands/operation/log.rs b/cli/src/commands/operation/log.rs index 5f5242c7f..d89915424 100644 --- a/cli/src/commands/operation/log.rs +++ b/cli/src/commands/operation/log.rs @@ -15,6 +15,7 @@ use std::slice; use itertools::Itertools as _; +use jj_lib::config::ConfigError; use jj_lib::op_walk; use jj_lib::operation::Operation; use jj_lib::repo::RepoLoader; @@ -239,10 +240,7 @@ fn do_op_log( Ok(()) } -fn get_node_template( - style: GraphStyle, - settings: &UserSettings, -) -> Result { +fn get_node_template(style: GraphStyle, settings: &UserSettings) -> Result { let symbol = settings .config() .get_string("templates.op_log_node") diff --git a/cli/src/config.rs b/cli/src/config.rs index 824ea5b6f..fcb2ce8d9 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -22,6 +22,7 @@ use std::path::PathBuf; use std::process::Command; use itertools::Itertools; +use jj_lib::config::ConfigError; use jj_lib::config::ConfigNamePathBuf; use jj_lib::settings::ConfigResultExt as _; use regex::Captures; @@ -47,9 +48,9 @@ pub fn parse_toml_value_or_bare_string(value_str: &str) -> toml_edit::Value { } } -pub fn to_toml_value(value: &config::Value) -> Result { - fn type_error(message: T) -> config::ConfigError { - config::ConfigError::Message(message.to_string()) +pub fn to_toml_value(value: &config::Value) -> Result { + fn type_error(message: T) -> ConfigError { + ConfigError::Message(message.to_string()) } // It's unlikely that the config object contained unsupported values, but // there's no guarantee. For example, values coming from environment @@ -76,7 +77,7 @@ pub fn to_toml_value(value: &config::Value) -> Result config::Config { builder.build().unwrap() } -fn read_config_file(path: &Path) -> Result { +fn read_config_file(path: &Path) -> Result { config::Config::builder() .add_source( config::File::from(path) @@ -449,7 +450,7 @@ fn read_config_file(path: &Path) -> Result .build() } -fn read_config_path(config_path: &Path) -> Result { +fn read_config_path(config_path: &Path) -> Result { let mut files = vec![]; if config_path.is_dir() { if let Ok(read_dir) = config_path.read_dir() { @@ -556,11 +557,11 @@ pub fn remove_config_value_from_file( let target_table = key_iter.try_fold(doc.as_table_mut(), |table, key| { table .get_mut(key) - .ok_or_else(|| config::ConfigError::NotFound(key.to_string())) + .ok_or_else(|| ConfigError::NotFound(key.to_string())) .and_then(|table| { - table.as_table_mut().ok_or_else(|| { - config::ConfigError::Message(format!(r#""{key}" is not a table"#)) - }) + table + .as_table_mut() + .ok_or_else(|| ConfigError::Message(format!(r#""{key}" is not a table"#))) }) })?; @@ -573,7 +574,7 @@ pub fn remove_config_value_from_file( entry.remove(); } toml_edit::Entry::Vacant(_) => { - return Err(config::ConfigError::NotFound(key.to_string()).into()); + return Err(ConfigError::NotFound(key.to_string()).into()); } } diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 06f3931af..c61f36451 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -32,6 +32,7 @@ use jj_lib::backend::CommitId; use jj_lib::backend::CopyRecord; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; +use jj_lib::config::ConfigError; use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::conflicts::materialized_diff_stream; use jj_lib::conflicts::MaterializedTreeDiffEntry; @@ -145,7 +146,7 @@ pub enum DiffFormat { pub fn diff_formats_for( settings: &UserSettings, args: &DiffFormatArgs, -) -> Result, config::ConfigError> { +) -> Result, ConfigError> { let formats = diff_formats_from_args(settings, args)?; if formats.is_empty() { Ok(vec![default_diff_format(settings, args)?]) @@ -160,7 +161,7 @@ pub fn diff_formats_for_log( settings: &UserSettings, args: &DiffFormatArgs, patch: bool, -) -> Result, config::ConfigError> { +) -> Result, ConfigError> { let mut formats = diff_formats_from_args(settings, args)?; // --patch implies default if no format other than --summary is specified if patch && matches!(formats.as_slice(), [] | [DiffFormat::Summary]) { @@ -173,7 +174,7 @@ pub fn diff_formats_for_log( fn diff_formats_from_args( settings: &UserSettings, args: &DiffFormatArgs, -) -> Result, config::ConfigError> { +) -> Result, ConfigError> { let mut formats = Vec::new(); if args.summary { formats.push(DiffFormat::Summary); @@ -207,7 +208,7 @@ fn diff_formats_from_args( fn default_diff_format( settings: &UserSettings, args: &DiffFormatArgs, -) -> Result { +) -> Result { let config = settings.config(); if let Some(args) = config.get("ui.diff.tool").optional()? { // External "tool" overrides the internal "format" option. @@ -242,9 +243,7 @@ fn default_diff_format( let options = DiffStatOptions::from_args(args); Ok(DiffFormat::Stat(Box::new(options))) } - _ => Err(config::ConfigError::Message(format!( - "invalid diff format: {name}" - ))), + _ => Err(ConfigError::Message(format!("invalid diff format: {name}"))), } } @@ -512,15 +511,16 @@ impl ColorWordsDiffOptions { fn from_settings_and_args( settings: &UserSettings, args: &DiffFormatArgs, - ) -> Result { + ) -> Result { let config = settings.config(); let max_inline_alternation = { let key = "diff.color-words.max-inline-alternation"; match config.get_int(key)? { -1 => None, // unlimited - n => Some(usize::try_from(n).map_err(|err| { - config::ConfigError::Message(format!("invalid {key}: {err}")) - })?), + n => Some( + usize::try_from(n) + .map_err(|err| ConfigError::Message(format!("invalid {key}: {err}")))?, + ), } }; let context = args @@ -1213,7 +1213,7 @@ impl UnifiedDiffOptions { fn from_settings_and_args( settings: &UserSettings, args: &DiffFormatArgs, - ) -> Result { + ) -> Result { let context = args .context .map_or_else(|| settings.config().get("diff.git.context"), Ok)?; diff --git a/cli/src/formatter.rs b/cli/src/formatter.rs index f6d27ce49..e19286d96 100644 --- a/cli/src/formatter.rs +++ b/cli/src/formatter.rs @@ -29,6 +29,7 @@ use crossterm::style::SetAttribute; use crossterm::style::SetBackgroundColor; use crossterm::style::SetForegroundColor; use itertools::Itertools; +use jj_lib::config::ConfigError; // Lets the caller label strings and translates the labels to colors pub trait Formatter: Write { @@ -158,7 +159,7 @@ impl FormatterFactory { FormatterFactory { kind } } - pub fn color(config: &config::Config, debug: bool) -> Result { + pub fn color(config: &config::Config, debug: bool) -> Result { let rules = Arc::new(rules_from_config(config)?); let kind = FormatterFactoryKind::Color { rules, debug }; Ok(FormatterFactory { kind }) @@ -299,7 +300,7 @@ impl ColorFormatter { output: W, config: &config::Config, debug: bool, - ) -> Result { + ) -> Result { let rules = rules_from_config(config)?; Ok(Self::new(output, Arc::new(rules), debug)) } @@ -403,7 +404,7 @@ impl ColorFormatter { } } -fn rules_from_config(config: &config::Config) -> Result { +fn rules_from_config(config: &config::Config) -> Result { let mut result = vec![]; let table = config.get_table("colors")?; for (key, value) in table { @@ -451,7 +452,7 @@ fn rules_from_config(config: &config::Config) -> Result Result { +fn color_for_name_or_hex(name_or_hex: &str) -> Result { match name_or_hex { "default" => Ok(Color::Reset), "black" => Ok(Color::Black), @@ -471,7 +472,7 @@ fn color_for_name_or_hex(name_or_hex: &str) -> Result Ok(Color::Cyan), "bright white" => Ok(Color::White), _ => color_for_hex(name_or_hex) - .ok_or_else(|| config::ConfigError::Message(format!("invalid color: {name_or_hex}"))), + .ok_or_else(|| ConfigError::Message(format!("invalid color: {name_or_hex}"))), } } diff --git a/cli/src/graphlog.rs b/cli/src/graphlog.rs index fc0fb0cb2..1d7d72aa8 100644 --- a/cli/src/graphlog.rs +++ b/cli/src/graphlog.rs @@ -17,6 +17,7 @@ use std::io; use std::io::Write; use itertools::Itertools; +use jj_lib::config::ConfigError; use jj_lib::settings::UserSettings; use renderdag::Ancestor; use renderdag::GraphRowRenderer; @@ -112,7 +113,7 @@ pub enum GraphStyle { } impl GraphStyle { - pub fn from_settings(settings: &UserSettings) -> Result { + pub fn from_settings(settings: &UserSettings) -> Result { settings.config().get("ui.graph.style") } diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 18cf8546b..c507e1d7a 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -18,8 +18,8 @@ mod external; use std::sync::Arc; -use config::ConfigError; use jj_lib::backend::MergedTreeId; +use jj_lib::config::ConfigError; use jj_lib::conflicts::extract_as_single_hunk; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::matchers::Matcher; @@ -60,7 +60,7 @@ pub enum DiffEditError { #[error("Failed to snapshot changes")] Snapshot(#[from] SnapshotError), #[error(transparent)] - Config(#[from] config::ConfigError), + Config(#[from] ConfigError), } #[derive(Debug, Error)] diff --git a/cli/src/ui.rs b/cli/src/ui.rs index b57bf4d90..1bd935d75 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -32,6 +32,7 @@ use std::thread::JoinHandle; use indoc::indoc; use itertools::Itertools as _; +use jj_lib::config::ConfigError; use minus::MinusError; use minus::Pager as MinusPager; use tracing::instrument; @@ -311,7 +312,7 @@ fn color_setting(config: &config::Config) -> ColorChoice { fn prepare_formatter_factory( config: &config::Config, stdout: &Stdout, -) -> Result { +) -> Result { let terminal = stdout.is_terminal(); let (color, debug) = match color_setting(config) { ColorChoice::Always => (true, false), diff --git a/lib/src/config.rs b/lib/src/config.rs index 743babd16..9ad580dcc 100644 --- a/lib/src/config.rs +++ b/lib/src/config.rs @@ -22,6 +22,10 @@ use std::str::FromStr; use config::Source as _; use itertools::Itertools as _; +/// Error that can occur when accessing configuration. +// TODO: will be replaced with our custom error type +pub type ConfigError = config::ConfigError; + /// Dotted config name path. #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub struct ConfigNamePathBuf(Vec); @@ -54,10 +58,7 @@ impl ConfigNamePathBuf { /// This is a workaround for the `config.get()` API, which doesn't support /// literal path expression. If we implement our own config abstraction, /// this method should be moved there. - pub fn lookup_value( - &self, - config: &config::Config, - ) -> Result { + pub fn lookup_value(&self, config: &config::Config) -> Result { // Use config.get() if the TOML keys can be converted to config path // syntax. This should be cheaper than cloning the whole config map. let (key_prefix, components) = self.split_safe_prefix(); @@ -71,7 +72,7 @@ impl ConfigNamePathBuf { let mut table = value.into_table().ok()?; table.remove(key.get()) }) - .ok_or_else(|| config::ConfigError::NotFound(self.to_string())) + .ok_or_else(|| ConfigError::NotFound(self.to_string())) } /// Splits path to dotted literal expression and remainder. diff --git a/lib/src/fsmonitor.rs b/lib/src/fsmonitor.rs index 4cf93f407..1cf4317b9 100644 --- a/lib/src/fsmonitor.rs +++ b/lib/src/fsmonitor.rs @@ -25,8 +25,8 @@ use std::path::PathBuf; use config::Config; -use config::ConfigError; +use crate::config::ConfigError; use crate::settings::ConfigResultExt; /// Config for Watchman filesystem monitor (). diff --git a/lib/src/gpg_signing.rs b/lib/src/gpg_signing.rs index 23603faed..308a0d5c3 100644 --- a/lib/src/gpg_signing.rs +++ b/lib/src/gpg_signing.rs @@ -25,6 +25,7 @@ use std::str; use thiserror::Error; +use crate::config::ConfigError; use crate::settings::ConfigResultExt as _; use crate::settings::UserSettings; use crate::signing::SigStatus; @@ -145,7 +146,7 @@ impl GpgBackend { self } - pub fn from_settings(settings: &UserSettings) -> Result { + pub fn from_settings(settings: &UserSettings) -> Result { let program = settings .config() .get_string("signing.backends.gpg.program") diff --git a/lib/src/settings.rs b/lib/src/settings.rs index 6158debc8..dab6ec1e5 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -26,6 +26,7 @@ use crate::backend::ChangeId; use crate::backend::Commit; use crate::backend::Signature; use crate::backend::Timestamp; +use crate::config::ConfigError; use crate::fmt_util::binary_prefix; use crate::fsmonitor::FsmonitorSettings; use crate::signing::SignBehavior; @@ -147,7 +148,7 @@ impl UserSettings { // TODO: Reconsider UserSettings/RepoSettings abstraction. See // https://github.com/martinvonz/jj/issues/616#issuecomment-1345170699 - pub fn with_repo(&self, _repo_path: &Path) -> Result { + pub fn with_repo(&self, _repo_path: &Path) -> Result { let config = self.config.clone(); Ok(RepoSettings { _config: config }) } @@ -167,7 +168,7 @@ impl UserSettings { self.config.get_string("user.email").unwrap_or_default() } - pub fn fsmonitor_settings(&self) -> Result { + pub fn fsmonitor_settings(&self) -> Result { FsmonitorSettings::from_config(&self.config) } @@ -238,7 +239,7 @@ impl UserSettings { GitSettings::from_config(&self.config) } - pub fn max_new_file_size(&self) -> Result { + pub fn max_new_file_size(&self) -> Result { let cfg = self .config .get::("snapshot.max-new-file-size") @@ -246,7 +247,7 @@ impl UserSettings { match cfg { Ok(0) => Ok(u64::MAX), x @ Ok(_) => x, - Err(config::ConfigError::NotFound(_)) => Ok(1024 * 1024), + Err(ConfigError::NotFound(_)) => Ok(1024 * 1024), e @ Err(_) => e, } } @@ -290,14 +291,14 @@ impl JJRng { } pub trait ConfigResultExt { - fn optional(self) -> Result, config::ConfigError>; + fn optional(self) -> Result, ConfigError>; } -impl ConfigResultExt for Result { - fn optional(self) -> Result, config::ConfigError> { +impl ConfigResultExt for Result { + fn optional(self) -> Result, ConfigError> { match self { Ok(value) => Ok(Some(value)), - Err(config::ConfigError::NotFound(_)) => Ok(None), + Err(ConfigError::NotFound(_)) => Ok(None), Err(err) => Err(err), } } diff --git a/lib/src/signing.rs b/lib/src/signing.rs index 7a50cc5ed..d0d594988 100644 --- a/lib/src/signing.rs +++ b/lib/src/signing.rs @@ -22,6 +22,7 @@ use std::sync::RwLock; use thiserror::Error; use crate::backend::CommitId; +use crate::config::ConfigError; use crate::gpg_signing::GpgBackend; use crate::settings::UserSettings; use crate::ssh_signing::SshBackend; @@ -121,7 +122,7 @@ pub enum SignInitError { UnknownBackend(String), /// Failed to load backend configuration. #[error("Failed to configure signing backend")] - BackendConfig(#[source] config::ConfigError), + BackendConfig(#[source] ConfigError), } /// A enum that describes if a created/rewritten commit should be signed or not. diff --git a/lib/src/ssh_signing.rs b/lib/src/ssh_signing.rs index fad1cda59..e2367209d 100644 --- a/lib/src/ssh_signing.rs +++ b/lib/src/ssh_signing.rs @@ -26,6 +26,7 @@ use std::process::Stdio; use either::Either; use thiserror::Error; +use crate::config::ConfigError; use crate::settings::ConfigResultExt as _; use crate::settings::UserSettings; use crate::signing::SigStatus; @@ -118,7 +119,7 @@ impl SshBackend { } } - pub fn from_settings(settings: &UserSettings) -> Result { + pub fn from_settings(settings: &UserSettings) -> Result { let program = settings .config() .get_string("signing.backends.ssh.program")