From 5ad0f3dcf6dc673c55d31c0b5fdee5eb65f9bcbb Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 22 Nov 2024 10:48:43 +0900 Subject: [PATCH] config: extract ConfigNamePathBuf to jj-lib I'm planning to rewrite config store layer by leveraging toml_edit instead of the config crate. It will allow us to merge config overlays in a way that deprecated keys are resolved within a layer prior to merging, for example. This patch moves ConfigNamePathBuf to jj-lib where new config API will be hosted. We'll probably extract LayeredConfigs to this module, but we'll first need to split environment dependencies from it. --- Cargo.lock | 1 + cli/src/cli_util.rs | 2 +- cli/src/commands/config/get.rs | 2 +- cli/src/commands/config/list.rs | 2 +- cli/src/commands/config/set.rs | 2 +- cli/src/commands/config/unset.rs | 2 +- cli/src/commands/git/clone.rs | 2 +- cli/src/commands/git/init.rs | 2 +- cli/src/complete.rs | 2 +- cli/src/config.rs | 146 +------------------------- lib/Cargo.toml | 1 + lib/src/config.rs | 170 +++++++++++++++++++++++++++++++ lib/src/lib.rs | 1 + 13 files changed, 182 insertions(+), 153 deletions(-) create mode 100644 lib/src/config.rs diff --git a/Cargo.lock b/Cargo.lock index 7fb388f37..e5c5aa8cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1913,6 +1913,7 @@ dependencies = [ "testutils", "thiserror", "tokio", + "toml_edit", "tracing", "version_check", "watchman_client", diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index dbafda10c..a445d5620 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::ConfigNamePathBuf; use jj_lib::file_util; use jj_lib::fileset; use jj_lib::fileset::FilesetDiagnostics; @@ -145,7 +146,6 @@ use crate::complete; use crate::config::new_config_path; use crate::config::AnnotatedValue; use crate::config::CommandNameAndArgs; -use crate::config::ConfigNamePathBuf; use crate::config::ConfigSource; use crate::config::LayeredConfigs; use crate::diff_util; diff --git a/cli/src/commands/config/get.rs b/cli/src/commands/config/get.rs index 70184cf78..f5a97b351 100644 --- a/cli/src/commands/config/get.rs +++ b/cli/src/commands/config/get.rs @@ -15,13 +15,13 @@ use std::io::Write as _; use clap_complete::ArgValueCandidates; +use jj_lib::config::ConfigNamePathBuf; use tracing::instrument; use crate::cli_util::CommandHelper; use crate::command_error::config_error; use crate::command_error::CommandError; use crate::complete; -use crate::config::ConfigNamePathBuf; use crate::ui::Ui; /// Get the value of a given config option. diff --git a/cli/src/commands/config/list.rs b/cli/src/commands/config/list.rs index e4501a40f..f2812217d 100644 --- a/cli/src/commands/config/list.rs +++ b/cli/src/commands/config/list.rs @@ -13,6 +13,7 @@ // limitations under the License. use clap_complete::ArgValueCandidates; +use jj_lib::config::ConfigNamePathBuf; use tracing::instrument; use super::ConfigLevelArgs; @@ -21,7 +22,6 @@ use crate::command_error::CommandError; use crate::complete; use crate::config::to_toml_value; use crate::config::AnnotatedValue; -use crate::config::ConfigNamePathBuf; use crate::config::ConfigSource; use crate::generic_templater::GenericTemplateLanguage; use crate::template_builder::TemplateLanguage as _; diff --git a/cli/src/commands/config/set.rs b/cli/src/commands/config/set.rs index ff4f816c5..0167b4edc 100644 --- a/cli/src/commands/config/set.rs +++ b/cli/src/commands/config/set.rs @@ -16,6 +16,7 @@ use std::io; use clap_complete::ArgValueCandidates; use jj_lib::commit::Commit; +use jj_lib::config::ConfigNamePathBuf; use jj_lib::repo::Repo; use tracing::instrument; @@ -28,7 +29,6 @@ use crate::command_error::CommandError; use crate::complete; use crate::config::parse_toml_value_or_bare_string; use crate::config::write_config_value_to_file; -use crate::config::ConfigNamePathBuf; use crate::ui::Ui; /// Update config file to set the given option to a given value. diff --git a/cli/src/commands/config/unset.rs b/cli/src/commands/config/unset.rs index 1b8400c7a..84b99575f 100644 --- a/cli/src/commands/config/unset.rs +++ b/cli/src/commands/config/unset.rs @@ -13,6 +13,7 @@ // limitations under the License. use clap_complete::ArgValueCandidates; +use jj_lib::config::ConfigNamePathBuf; use tracing::instrument; use super::ConfigLevelArgs; @@ -22,7 +23,6 @@ use crate::command_error::user_error; use crate::command_error::CommandError; use crate::complete; use crate::config::remove_config_value_from_file; -use crate::config::ConfigNamePathBuf; use crate::ui::Ui; /// Update config file to unset the given option. diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs index f9e148882..57f026108 100644 --- a/cli/src/commands/git/clone.rs +++ b/cli/src/commands/git/clone.rs @@ -19,6 +19,7 @@ use std::num::NonZeroU32; use std::path::Path; use std::path::PathBuf; +use jj_lib::config::ConfigNamePathBuf; use jj_lib::git; use jj_lib::git::GitFetchError; use jj_lib::git::GitFetchStats; @@ -34,7 +35,6 @@ use crate::command_error::user_error_with_message; use crate::command_error::CommandError; use crate::commands::git::maybe_add_gitignore; use crate::config::write_config_value_to_file; -use crate::config::ConfigNamePathBuf; use crate::git_util::get_git_repo; use crate::git_util::map_git_error; use crate::git_util::print_git_import_stats; diff --git a/cli/src/commands/git/init.rs b/cli/src/commands/git/init.rs index 1fc8ba539..7677fbb78 100644 --- a/cli/src/commands/git/init.rs +++ b/cli/src/commands/git/init.rs @@ -17,6 +17,7 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use jj_lib::config::ConfigNamePathBuf; use jj_lib::file_util; use jj_lib::git; use jj_lib::git::parse_git_ref; @@ -35,7 +36,6 @@ use crate::command_error::user_error_with_message; use crate::command_error::CommandError; use crate::commands::git::maybe_add_gitignore; use crate::config::write_config_value_to_file; -use crate::config::ConfigNamePathBuf; use crate::git_util::get_git_repo; use crate::git_util::is_colocated_git_workspace; use crate::git_util::print_failed_git_export; diff --git a/cli/src/complete.rs b/cli/src/complete.rs index 6418849af..56d7e8723 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -17,6 +17,7 @@ use clap::FromArgMatches as _; use clap_complete::CompletionCandidate; use config::Config; use itertools::Itertools; +use jj_lib::config::ConfigNamePathBuf; use jj_lib::workspace::DefaultWorkspaceLoaderFactory; use jj_lib::workspace::WorkspaceLoaderFactory as _; @@ -26,7 +27,6 @@ use crate::cli_util::GlobalArgs; use crate::command_error::user_error; use crate::command_error::CommandError; use crate::config::default_config; -use crate::config::ConfigNamePathBuf; use crate::config::LayeredConfigs; use crate::config::CONFIG_SCHEMA; use crate::ui::Ui; diff --git a/cli/src/config.rs b/cli/src/config.rs index bb2fa5406..19f44bf13 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -20,11 +20,9 @@ use std::fmt; use std::path::Path; use std::path::PathBuf; use std::process::Command; -use std::slice; -use std::str::FromStr; -use config::Source; use itertools::Itertools; +use jj_lib::config::ConfigNamePathBuf; use jj_lib::settings::ConfigResultExt as _; use regex::Captures; use regex::Regex; @@ -85,111 +83,6 @@ pub enum ConfigError { ConfigCreateError(#[from] std::io::Error), } -/// Dotted config name path. -#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] -pub struct ConfigNamePathBuf(Vec); - -impl ConfigNamePathBuf { - /// Creates an empty path pointing to the root table. - /// - /// This isn't a valid TOML key expression, but provided for convenience. - pub fn root() -> Self { - ConfigNamePathBuf(vec![]) - } - - /// Returns true if the path is empty (i.e. pointing to the root table.) - pub fn is_root(&self) -> bool { - self.0.is_empty() - } - - /// Returns iterator of path components (or keys.) - pub fn components(&self) -> slice::Iter<'_, toml_edit::Key> { - self.0.iter() - } - - /// Appends the given `key` component. - pub fn push(&mut self, key: impl Into) { - self.0.push(key.into()); - } - - /// Looks up value in the given `config`. - /// - /// 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 { - // 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(); - let value: config::Value = match &key_prefix { - Some(key) => config.get(key)?, - None => config.collect()?.into(), - }; - components - .iter() - .try_fold(value, |value, key| { - let mut table = value.into_table().ok()?; - table.remove(key.get()) - }) - .ok_or_else(|| config::ConfigError::NotFound(self.to_string())) - } - - /// Splits path to dotted literal expression and remainder. - /// - /// The literal expression part doesn't contain meta characters other than - /// ".", therefore it can be passed in to `config.get()`. - /// https://github.com/mehcode/config-rs/issues/110 - fn split_safe_prefix(&self) -> (Option>, &[toml_edit::Key]) { - // https://github.com/mehcode/config-rs/blob/v0.13.4/src/path/parser.rs#L15 - let is_ident = |key: &str| { - key.chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') - }; - let pos = self.0.iter().take_while(|&k| is_ident(k)).count(); - let safe_key = match pos { - 0 => None, - 1 => Some(Cow::Borrowed(self.0[0].get())), - _ => Some(Cow::Owned(self.0[..pos].iter().join("."))), - }; - (safe_key, &self.0[pos..]) - } -} - -impl> FromIterator for ConfigNamePathBuf { - fn from_iter>(iter: I) -> Self { - let keys = iter.into_iter().map(|k| k.into()).collect(); - ConfigNamePathBuf(keys) - } -} - -impl FromStr for ConfigNamePathBuf { - type Err = toml_edit::TomlError; - - fn from_str(s: &str) -> Result { - // TOML parser ensures that the returned vec is not empty. - toml_edit::Key::parse(s).map(ConfigNamePathBuf) - } -} - -impl AsRef<[toml_edit::Key]> for ConfigNamePathBuf { - fn as_ref(&self) -> &[toml_edit::Key] { - &self.0 - } -} - -impl fmt::Display for ConfigNamePathBuf { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut components = self.0.iter().fuse(); - if let Some(key) = components.next() { - write!(f, "{key}")?; - } - components.try_for_each(|key| write!(f, ".{key}")) - } -} - #[derive(Clone, Debug, PartialEq, Eq)] pub enum ConfigSource { Default, @@ -829,43 +722,6 @@ mod tests { use super::*; - #[test] - fn test_split_safe_config_name_path() { - let parse = |s| ConfigNamePathBuf::from_str(s).unwrap(); - let key = |s: &str| toml_edit::Key::new(s); - - // Empty (or root) path isn't recognized by config::Config::get() - assert_eq!( - ConfigNamePathBuf::root().split_safe_prefix(), - (None, [].as_slice()) - ); - - assert_eq!( - parse("Foo-bar_1").split_safe_prefix(), - (Some("Foo-bar_1".into()), [].as_slice()) - ); - assert_eq!( - parse("'foo()'").split_safe_prefix(), - (None, [key("foo()")].as_slice()) - ); - assert_eq!( - parse("foo.'bar()'").split_safe_prefix(), - (Some("foo".into()), [key("bar()")].as_slice()) - ); - assert_eq!( - parse("foo.'bar()'.baz").split_safe_prefix(), - (Some("foo".into()), [key("bar()"), key("baz")].as_slice()) - ); - assert_eq!( - parse("foo.bar").split_safe_prefix(), - (Some("foo.bar".into()), [].as_slice()) - ); - assert_eq!( - parse("foo.bar.'baz()'").split_safe_prefix(), - (Some("foo.bar".into()), [key("baz()")].as_slice()) - ); - } - #[test] fn test_command_args() { let config = config::Config::builder() diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 53b035ab6..aca359958 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -71,6 +71,7 @@ strsim = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, optional = true } +toml_edit = { workspace = true } tracing = { workspace = true } watchman_client = { workspace = true, optional = true } whoami = { workspace = true } diff --git a/lib/src/config.rs b/lib/src/config.rs new file mode 100644 index 000000000..743babd16 --- /dev/null +++ b/lib/src/config.rs @@ -0,0 +1,170 @@ +// Copyright 2022 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Configuration store helpers. + +use std::borrow::Cow; +use std::fmt; +use std::slice; +use std::str::FromStr; + +use config::Source as _; +use itertools::Itertools as _; + +/// Dotted config name path. +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct ConfigNamePathBuf(Vec); + +impl ConfigNamePathBuf { + /// Creates an empty path pointing to the root table. + /// + /// This isn't a valid TOML key expression, but provided for convenience. + pub fn root() -> Self { + ConfigNamePathBuf(vec![]) + } + + /// Returns true if the path is empty (i.e. pointing to the root table.) + pub fn is_root(&self) -> bool { + self.0.is_empty() + } + + /// Returns iterator of path components (or keys.) + pub fn components(&self) -> slice::Iter<'_, toml_edit::Key> { + self.0.iter() + } + + /// Appends the given `key` component. + pub fn push(&mut self, key: impl Into) { + self.0.push(key.into()); + } + + /// Looks up value in the given `config`. + /// + /// 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 { + // 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(); + let value: config::Value = match &key_prefix { + Some(key) => config.get(key)?, + None => config.collect()?.into(), + }; + components + .iter() + .try_fold(value, |value, key| { + let mut table = value.into_table().ok()?; + table.remove(key.get()) + }) + .ok_or_else(|| config::ConfigError::NotFound(self.to_string())) + } + + /// Splits path to dotted literal expression and remainder. + /// + /// The literal expression part doesn't contain meta characters other than + /// ".", therefore it can be passed in to `config.get()`. + /// https://github.com/mehcode/config-rs/issues/110 + fn split_safe_prefix(&self) -> (Option>, &[toml_edit::Key]) { + // https://github.com/mehcode/config-rs/blob/v0.13.4/src/path/parser.rs#L15 + let is_ident = |key: &str| { + key.chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + }; + let pos = self.0.iter().take_while(|&k| is_ident(k)).count(); + let safe_key = match pos { + 0 => None, + 1 => Some(Cow::Borrowed(self.0[0].get())), + _ => Some(Cow::Owned(self.0[..pos].iter().join("."))), + }; + (safe_key, &self.0[pos..]) + } +} + +impl> FromIterator for ConfigNamePathBuf { + fn from_iter>(iter: I) -> Self { + let keys = iter.into_iter().map(|k| k.into()).collect(); + ConfigNamePathBuf(keys) + } +} + +impl FromStr for ConfigNamePathBuf { + type Err = toml_edit::TomlError; + + fn from_str(s: &str) -> Result { + // TOML parser ensures that the returned vec is not empty. + toml_edit::Key::parse(s).map(ConfigNamePathBuf) + } +} + +impl AsRef<[toml_edit::Key]> for ConfigNamePathBuf { + fn as_ref(&self) -> &[toml_edit::Key] { + &self.0 + } +} + +impl fmt::Display for ConfigNamePathBuf { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut components = self.0.iter().fuse(); + if let Some(key) = components.next() { + write!(f, "{key}")?; + } + components.try_for_each(|key| write!(f, ".{key}")) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_split_safe_config_name_path() { + let parse = |s| ConfigNamePathBuf::from_str(s).unwrap(); + let key = |s: &str| toml_edit::Key::new(s); + + // Empty (or root) path isn't recognized by config::Config::get() + assert_eq!( + ConfigNamePathBuf::root().split_safe_prefix(), + (None, [].as_slice()) + ); + + assert_eq!( + parse("Foo-bar_1").split_safe_prefix(), + (Some("Foo-bar_1".into()), [].as_slice()) + ); + assert_eq!( + parse("'foo()'").split_safe_prefix(), + (None, [key("foo()")].as_slice()) + ); + assert_eq!( + parse("foo.'bar()'").split_safe_prefix(), + (Some("foo".into()), [key("bar()")].as_slice()) + ); + assert_eq!( + parse("foo.'bar()'.baz").split_safe_prefix(), + (Some("foo".into()), [key("bar()"), key("baz")].as_slice()) + ); + assert_eq!( + parse("foo.bar").split_safe_prefix(), + (Some("foo.bar".into()), [].as_slice()) + ); + assert_eq!( + parse("foo.bar.'baz()'").split_safe_prefix(), + (Some("foo.bar".into()), [key("baz()")].as_slice()) + ); + } +} diff --git a/lib/src/lib.rs b/lib/src/lib.rs index d629ba88c..baf80ff26 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -32,6 +32,7 @@ pub mod annotate; pub mod backend; pub mod commit; pub mod commit_builder; +pub mod config; pub mod conflicts; pub mod copies; pub mod dag_walk;