From 84a34bada98263ec7ceae7dcdec44fbe39394057 Mon Sep 17 00:00:00 2001 From: Marijan Smetko Date: Sun, 21 Jul 2024 00:48:56 +0200 Subject: [PATCH] Warn user about the working copy when configuring the author --- CHANGELOG.md | 2 + cli/src/commands/config/set.rs | 89 ++++++++++++++++++++++++++++++-- cli/tests/test_config_command.rs | 24 +++++++++ 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbd7da8988a..d1a1e58229c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * `jj describe` can now update the description of multiple commits. +* When reconfiguring the author, warn that the working copy won't be updated + ### Fixed bugs * `jj status` will show different messages in a conflicted tree, depending diff --git a/cli/src/commands/config/set.rs b/cli/src/commands/config/set.rs index cfa8106c2dc..cb265da28d3 100644 --- a/cli/src/commands/config/set.rs +++ b/cli/src/commands/config/set.rs @@ -12,16 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. +use jj_lib::commit::Commit; +use jj_lib::repo::Repo; use tracing::instrument; use super::ConfigLevelArgs; -use crate::cli_util::{get_new_config_file_path, CommandHelper}; +use crate::cli_util::{get_new_config_file_path, CommandHelper, WorkspaceCommandHelper}; use crate::command_error::{user_error, CommandError}; use crate::config::{ parse_toml_value_or_bare_string, write_config_value_to_file, ConfigNamePathBuf, }; use crate::ui::Ui; - /// Update config file to set the given option to a given value. #[derive(clap::Args, Clone, Debug)] pub struct ConfigSetArgs { @@ -34,8 +35,8 @@ pub struct ConfigSetArgs { } #[instrument(skip_all)] -pub fn cmd_config_set( - _ui: &mut Ui, +pub(crate) fn cmd_config_set( + ui: &mut Ui, command: &CommandHelper, args: &ConfigSetArgs, ) -> Result<(), CommandError> { @@ -46,7 +47,85 @@ pub fn cmd_config_set( path = config_path.display() ))); } - // TODO(#531): Infer types based on schema (w/ --type arg to override). let value = parse_toml_value_or_bare_string(&args.value); + // If the user is trying to change the author config, we should warn them that + // it won't affect the working copy author + if args.name == ConfigNamePathBuf::from_iter(vec!["user", "name"]) { + check_wc_user_name(command, ui, &value)?; + } else if args.name == ConfigNamePathBuf::from_iter(vec!["user", "email"]) { + check_wc_user_email(command, ui, &value)?; + }; + write_config_value_to_file(&args.name, value, &config_path) } + +/// Returns the commit of the working copy if it exists. +fn maybe_wc_commit(helper: &WorkspaceCommandHelper) -> Option { + let repo = helper.repo(); + let maybe_wc_commit = helper + .get_wc_commit_id() + .map(|id| repo.store().get_commit(id)) + .transpose() + .unwrap(); + maybe_wc_commit +} + +/// Check if the working copy author name matches the user's config value +/// If it doesn't, print a warning message +fn check_wc_user_name( + command: &CommandHelper, + ui: &mut Ui, + user_name: &toml_edit::Value, +) -> Result<(), CommandError> { + let helper = command.workspace_helper(ui)?; + if let Some(wc_commit) = maybe_wc_commit(&helper) { + let author = wc_commit.author(); + match user_name.as_str() { + None => { + return Ok(()); + } + Some(user_name) => { + if author.name != user_name { + warn_wc_author(&author.name, &author.email, ui)? + } + return Ok(()); + } + }; + } + Ok(()) +} +/// Check if the working copy author email matches the user's config value +/// If it doesn't, print a warning message +fn check_wc_user_email( + command: &CommandHelper, + ui: &mut Ui, + user_email: &toml_edit::Value, +) -> Result<(), CommandError> { + let helper = command.workspace_helper(ui)?; + if let Some(wc_commit) = maybe_wc_commit(&helper) { + let author = wc_commit.author(); + + match user_email.as_str() { + None => { + return Ok(()); + } + Some(user_email) => { + if author.email != user_email { + warn_wc_author(&author.name, &author.email, ui)? + } + return Ok(()); + } + }; + }; + Ok(()) +} + +/// Prints a warning message about the working copy to the user +fn warn_wc_author(user_name: &str, user_email: &str, ui: &mut Ui) -> Result<(), CommandError> { + Ok(writeln!( + ui.warning_default(), + "This setting will only impact future commits.\nThe author of the working copy will stay \ + \"{user_name} <{user_email}>\".\nTo change the working copy author, use \"jj describe \ + --reset-author --no-edit\"" + )?) +} diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index ca1c07adbd2..82865a77ed4 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -857,6 +857,30 @@ fn test_config_show_paths() { "###); } +#[test] +fn test_config_author_change_warning() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["config", "set", "--repo", "user.email", "'Foo'"], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Warning: This setting will only impact future commits. + The author of the working copy will stay "Test User ". + To change the working copy author, use "jj describe --reset-author --no-edit" + "###); + + let mut log_cmd = test_env.jj_cmd(&repo_path, &["describe", "--reset-author", "--no-edit"]); + log_cmd.env_remove("JJ_EMAIL"); + log_cmd.assert().success(); + + let (stdout, _) = test_env.jj_cmd_ok(&repo_path, &["log"]); + assert![dbg![stdout].contains("Foo")]; +} + fn find_stdout_lines(keyname_pattern: &str, stdout: &str) -> String { let key_line_re = Regex::new(&format!(r"(?m)^{keyname_pattern} = .*$")).unwrap(); key_line_re