diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index cb770db720..6e56852b11 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -3704,7 +3704,7 @@ impl CliRunner { } } - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config)?; let command_helper_data = CommandHelperData { app: self.app, cwd, diff --git a/cli/src/complete.rs b/cli/src/complete.rs index ae49f4637c..ecd7393e26 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -753,7 +753,7 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { cmd: current_exe, args: cmd_args, }; - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config)?; Ok((builder, settings)) } diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index ec42601c8a..0ee78662b8 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -373,7 +373,7 @@ mod tests { fn test_get_diff_editor_with_name() { let get = |name, config_text| { let config = config_from_string(config_text); - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config).unwrap(); DiffEditor::with_name( name, &settings, @@ -442,7 +442,7 @@ mod tests { let get = |text| { let config = config_from_string(text); let ui = Ui::with_config(&config).unwrap(); - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config).unwrap(); DiffEditor::from_settings( &ui, &settings, @@ -614,7 +614,7 @@ mod tests { fn test_get_merge_editor_with_name() { let get = |name, config_text| { let config = config_from_string(config_text); - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config).unwrap(); MergeEditor::with_name(name, &settings, ConflictMarkerStyle::Diff) .map(|editor| editor.tool) }; @@ -666,7 +666,7 @@ mod tests { let get = |text| { let config = config_from_string(text); let ui = Ui::with_config(&config).unwrap(); - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config).unwrap(); MergeEditor::from_settings(&ui, &settings, ConflictMarkerStyle::Diff) .map(|editor| editor.tool) }; diff --git a/cli/tests/test_revset_output.rs b/cli/tests/test_revset_output.rs index 11d6e9988e..1866c7bd02 100644 --- a/cli/tests/test_revset_output.rs +++ b/cli/tests/test_revset_output.rs @@ -740,7 +740,7 @@ fn test_revset_committer_date_with_time_zone() { test_env.jj_cmd_ok( &repo_path, &[ - "--config=debug.commit-timestamp='2023-01-25T11:30:00-05:00'", + "--config=debug.commit-timestamp=2023-01-25T11:30:00-05:00", "describe", "-m", "first", @@ -749,7 +749,7 @@ fn test_revset_committer_date_with_time_zone() { test_env.jj_cmd_ok( &repo_path, &[ - "--config=debug.commit-timestamp='2023-01-25T12:30:00-05:00'", + "--config=debug.commit-timestamp=2023-01-25T12:30:00-05:00", "new", "-m", "second", @@ -758,7 +758,7 @@ fn test_revset_committer_date_with_time_zone() { test_env.jj_cmd_ok( &repo_path, &[ - "--config=debug.commit-timestamp='2023-01-25T13:30:00-05:00'", + "--config=debug.commit-timestamp=2023-01-25T13:30:00-05:00", "new", "-m", "third", @@ -768,7 +768,7 @@ fn test_revset_committer_date_with_time_zone() { let mut log_commits_before_and_after = |committer_date: &str, now: &str, tz: &str| -> (String, String) { test_env.add_env_var("TZ", tz); - let config = format!("debug.commit-timestamp='{now}'"); + let config = format!("debug.commit-timestamp={now}"); let before_log = test_env.jj_cmd_success( &repo_path, &[ diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 083f8fa713..19a30cd13f 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -2222,6 +2222,6 @@ mod tests { // our UserSettings type comes from jj_lib (1). fn user_settings() -> UserSettings { let config = StackedConfig::empty(); - UserSettings::from_config(config) + UserSettings::from_config(config).unwrap() } } diff --git a/lib/src/settings.rs b/lib/src/settings.rs index e5351ba1db..bea875429a 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -42,7 +42,8 @@ use crate::signing::SignBehavior; #[derive(Debug, Clone)] pub struct UserSettings { config: StackedConfig, - timestamp: Option, + commit_timestamp: Option, + operation_timestamp: Option, rng: Arc, } @@ -123,26 +124,36 @@ impl SignSettings { } } -fn get_timestamp_config(config: &StackedConfig, key: &'static str) -> Option { - // TODO: Maybe switch to native TOML date-time type? - match config.get::(key) { - Ok(timestamp_str) => match DateTime::parse_from_rfc3339(×tamp_str) { - Ok(datetime) => Some(Timestamp::from_datetime(datetime)), - Err(_) => None, - }, - Err(_) => None, +fn to_timestamp(value: ConfigValue) -> Result> { + // Since toml_edit::Datetime isn't the date-time type used across our code + // base, we accept both string and date-time types. + if let Some(s) = value.as_str() { + Ok(Timestamp::from_datetime(DateTime::parse_from_rfc3339(s)?)) + } else if let Some(d) = value.as_datetime() { + // It's easier to re-parse the TOML date-time expression. + let s = d.to_string(); + Ok(Timestamp::from_datetime(DateTime::parse_from_rfc3339(&s)?)) + } else { + let ty = value.type_name(); + Err(format!("invalid type: {ty}, expected a date-time").into()) } } impl UserSettings { - pub fn from_config(config: StackedConfig) -> Self { - let timestamp = get_timestamp_config(&config, "debug.commit-timestamp"); - let rng_seed = config.get::("debug.randomness-seed").ok(); - UserSettings { + pub fn from_config(config: StackedConfig) -> Result { + let commit_timestamp = config + .get_value_with("debug.commit-timestamp", to_timestamp) + .optional()?; + let operation_timestamp = config + .get_value_with("debug.operation-timestamp", to_timestamp) + .optional()?; + let rng_seed = config.get::("debug.randomness-seed").optional()?; + Ok(UserSettings { config, - timestamp, + commit_timestamp, + operation_timestamp, rng: Arc::new(JJRng::new(rng_seed)), - } + }) } // TODO: Reconsider UserSettings/RepoSettings abstraction. See @@ -176,11 +187,11 @@ impl UserSettings { pub const USER_EMAIL_PLACEHOLDER: &'static str = "(no email configured)"; pub fn commit_timestamp(&self) -> Option { - self.timestamp + self.commit_timestamp } pub fn operation_timestamp(&self) -> Option { - get_timestamp_config(&self.config, "debug.operation-timestamp") + self.operation_timestamp } pub fn operation_hostname(&self) -> String { @@ -212,7 +223,7 @@ impl UserSettings { } pub fn signature(&self) -> Signature { - let timestamp = self.timestamp.unwrap_or_else(Timestamp::now); + let timestamp = self.commit_timestamp.unwrap_or_else(Timestamp::now); Signature { name: self.user_name(), email: self.user_email(), diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index d134feacbd..f4bc4331e4 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -177,7 +177,7 @@ fn test_rewrite(backend: TestRepoBackend) { ) .unwrap(), ); - let rewrite_settings = UserSettings::from_config(config); + let rewrite_settings = UserSettings::from_config(config).unwrap(); let mut tx = repo.start_transaction(&settings); let rewritten_commit = tx .repo_mut() @@ -221,7 +221,7 @@ fn test_rewrite(backend: TestRepoBackend) { #[test_case(TestRepoBackend::Local ; "local backend")] #[test_case(TestRepoBackend::Git ; "git backend")] fn test_rewrite_update_missing_user(backend: TestRepoBackend) { - let missing_user_settings = UserSettings::from_config(StackedConfig::empty()); + let missing_user_settings = UserSettings::from_config(StackedConfig::empty()).unwrap(); let test_repo = TestRepo::init_with_backend(backend); let repo = &test_repo.repo; @@ -251,7 +251,7 @@ fn test_rewrite_update_missing_user(backend: TestRepoBackend) { ) .unwrap(), ); - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config).unwrap(); let rewritten_commit = tx .repo_mut() .rewrite_commit(&settings, &initial_commit) @@ -278,7 +278,8 @@ fn test_rewrite_resets_author_timestamp(backend: TestRepoBackend) { // Create discardable commit let initial_timestamp = "2001-02-03T04:05:06+07:00"; - let settings = UserSettings::from_config(config_with_commit_timestamp(initial_timestamp)); + let settings = + UserSettings::from_config(config_with_commit_timestamp(initial_timestamp)).unwrap(); let mut tx = repo.start_transaction(&settings); let initial_commit = tx .repo_mut() @@ -297,7 +298,8 @@ fn test_rewrite_resets_author_timestamp(backend: TestRepoBackend) { // Rewrite discardable commit to no longer be discardable let new_timestamp_1 = "2002-03-04T05:06:07+08:00"; - let settings = UserSettings::from_config(config_with_commit_timestamp(new_timestamp_1)); + let settings = + UserSettings::from_config(config_with_commit_timestamp(new_timestamp_1)).unwrap(); let rewritten_commit_1 = tx .repo_mut() .rewrite_commit(&settings, &initial_commit) @@ -315,7 +317,8 @@ fn test_rewrite_resets_author_timestamp(backend: TestRepoBackend) { // Rewrite non-discardable commit let new_timestamp_2 = "2003-04-05T06:07:08+09:00"; - let settings = UserSettings::from_config(config_with_commit_timestamp(new_timestamp_2)); + let settings = + UserSettings::from_config(config_with_commit_timestamp(new_timestamp_2)).unwrap(); let rewritten_commit_2 = tx .repo_mut() .rewrite_commit(&settings, &rewritten_commit_1) diff --git a/lib/tests/test_init.rs b/lib/tests/test_init.rs index 6546622bff..c8ffb8579d 100644 --- a/lib/tests/test_init.rs +++ b/lib/tests/test_init.rs @@ -143,7 +143,7 @@ fn test_init_no_config_set(backend: TestRepoBackend) { // Test that we can create a repo without setting any config // TODO: Perhaps, StackedConfig::empty() will be replaced with ::default() // or something that includes the minimal configuration variables. - let settings = UserSettings::from_config(StackedConfig::empty()); + let settings = UserSettings::from_config(StackedConfig::empty()).unwrap(); let test_workspace = TestWorkspace::init_with_backend(&settings, backend); let repo = &test_workspace.repo; let wc_commit_id = repo diff --git a/lib/tests/test_operations.rs b/lib/tests/test_operations.rs index 750a218be4..fbcfbe29c2 100644 --- a/lib/tests/test_operations.rs +++ b/lib/tests/test_operations.rs @@ -427,11 +427,11 @@ fn stable_op_id_settings() -> UserSettings { config.add_layer( ConfigLayer::parse( ConfigSource::User, - "debug.operation-timestamp = '2001-02-03T04:05:06+07:00'", + "debug.operation-timestamp = 2001-02-03T04:05:06+07:00", ) .unwrap(), ); - UserSettings::from_config(config) + UserSettings::from_config(config).unwrap() } #[test] diff --git a/lib/tests/test_signing.rs b/lib/tests/test_signing.rs index cc999be21d..e9a6021347 100644 --- a/lib/tests/test_signing.rs +++ b/lib/tests/test_signing.rs @@ -30,7 +30,7 @@ fn user_settings(sign_all: bool) -> UserSettings { ) .unwrap(), ); - UserSettings::from_config(config) + UserSettings::from_config(config).unwrap() } fn someone_else() -> Signature { diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index 2dadf568e6..86c442cba9 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -123,7 +123,7 @@ pub fn base_user_config() -> StackedConfig { /// Returns new immutable settings object that includes fake user configuration /// needed to run basic operations. pub fn user_settings() -> UserSettings { - UserSettings::from_config(base_user_config()) + UserSettings::from_config(base_user_config()).unwrap() } #[derive(Debug)]