From 1f136be165747d75492635bd9cd8c9a12b7aeed9 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 | 66 +++++++++++++++++++++++++++++++- cli/tests/test_config_command.rs | 37 ++++++++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c00e5e8557..8ab9c6610a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * A tilde (`~`) at the start of the path will now be expanded to the user's home directory when configuring a `signing.key` for SSH commit signing. + +* When reconfiguring the author, warn that the working copy won't be updated ### Fixed bugs diff --git a/cli/src/commands/config/set.rs b/cli/src/commands/config/set.rs index cfa8106c2d..6e4bed58d1 100644 --- a/cli/src/commands/config/set.rs +++ b/cli/src/commands/config/set.rs @@ -12,10 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::io; + +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, @@ -33,9 +37,15 @@ pub struct ConfigSetArgs { level: ConfigLevelArgs, } +/// Denotes a type of author change +enum AuthorChange { + Name, + Email, +} + #[instrument(skip_all)] pub fn cmd_config_set( - _ui: &mut Ui, + ui: &mut Ui, command: &CommandHelper, args: &ConfigSetArgs, ) -> Result<(), CommandError> { @@ -46,7 +56,59 @@ 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_author(ui, command, &value, AuthorChange::Name)?; + } else if args.name == ConfigNamePathBuf::from_iter(vec!["user", "email"]) { + check_wc_author(ui, command, &value, AuthorChange::Email)?; + }; + 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 id = helper.get_wc_commit_id()?; + repo.store().get_commit(id).ok() +} + +/// Check if the working copy author name matches the user's config value +/// If it doesn't, print a warning message +fn check_wc_author( + ui: &mut Ui, + command: &CommandHelper, + new_value: &toml_edit::Value, + author_change: AuthorChange, +) -> io::Result<()> { + let helper = match command.workspace_helper(ui) { + Ok(helper) => helper, + Err(_) => return Ok(()), // config set should work even if cwd isn't a jj repo + }; + if let Some(wc_commit) = maybe_wc_commit(&helper) { + let author = wc_commit.author(); + let orig_value = match author_change { + AuthorChange::Name => &author.name, + AuthorChange::Email => &author.email, + }; + if new_value.as_str() != Some(orig_value) { + warn_wc_author(ui, &author.name, &author.email)? + } + } + Ok(()) +} + +/// Prints a warning message about the working copy to the user +fn warn_wc_author(ui: &Ui, user_name: &str, user_email: &str) -> io::Result<()> { + 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 ca1c07adbd..8833f012a4 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -857,6 +857,43 @@ 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" + "###); + + // test_env.jj_cmd resets state for every invocation + // for this test, the state (user.email) is needed + 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!(stdout.contains("Foo")); +} + +#[test] +fn test_config_author_change_warning_root_env() { + let mut test_env = TestEnvironment::default(); + let user_config_path = test_env.config_path().join("config.toml"); + test_env.set_config_path(user_config_path); + test_env.jj_cmd_success( + test_env.env_root(), + &["config", "set", "--user", "user.email", "'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