CommitBuilder: when rewriting commit, replace placeholder user/email

If a commit's author field has the placeholder user/email values
(i.e. "(no name configured)" and "(no email configured)"), and they
have now configured their email and username, they probably want us to
update the author field with the new information, so that's what this
patch does. Thanks to durin42@ for the suggestion on #322.

CHANGELOG.md

@@ -99,6 +99,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * `jj log` now accepts a `--reversed` option, which will show older commits
   first.
 
+* The "(no name/email configured)" placeholder value for name/email will now be
+  replaced if once you modify a commit after having configured your name/email.
+
 ### Fixed bugs
 
 * When rebasing a conflict where one side modified a file and the other side

lib/src/commit_builder.rs

@@ -66,6 +66,14 @@ impl CommitBuilder {
         let mut commit = predecessor.store_commit().clone();
         commit.predecessors = vec![predecessor.id().clone()];
         commit.committer = settings.signature();
+        // If the user had not configured a name and email before but now they have,
+        // update the the author fields with the new information.
+        if commit.author.name == UserSettings::user_name_placeholder() {
+            commit.author.name = commit.committer.name.clone();
+        }
+        if commit.author.email == UserSettings::user_email_placeholder() {
+            commit.author.email = commit.committer.email.clone();
+        }
         CommitBuilder {
             store: store.clone(),
             commit,

lib/src/settings.rs

@@ -56,13 +56,21 @@ impl UserSettings {
     pub fn user_name(&self) -> String {
         self.config
             .get_string("user.name")
-            .unwrap_or_else(|_| "(no name configured)".to_string())
+            .unwrap_or_else(|_| Self::user_name_placeholder().to_string())
+    }
+
+    pub fn user_name_placeholder() -> &'static str {
+        "(no name configured)"
     }
 
     pub fn user_email(&self) -> String {
         self.config
             .get_string("user.email")
-            .unwrap_or_else(|_| "(no email configured)".to_string())
+            .unwrap_or_else(|_| Self::user_email_placeholder().to_string())
+    }
+
+    pub fn user_email_placeholder() -> &'static str {
+        "(no email configured)"
     }
 
     pub fn push_branch_prefix(&self) -> String {

lib/tests/test_commit_builder.rs

@@ -153,6 +153,52 @@ fn test_rewrite(use_git: bool) {
     );
 }
 
+// An author field with the placeholder name/email should get filled in on
+// rewrite
+#[test_case(false ; "local backend")]
+#[test_case(true ; "git backend")]
+fn test_rewrite_update_missing_user(use_git: bool) {
+    let missing_user_settings =
+        UserSettings::from_config(config::Config::builder().build().unwrap());
+    let test_repo = TestRepo::init(use_git);
+    let repo = &test_repo.repo;
+    let store = repo.store().clone();
+
+    let mut tx = repo.start_transaction("test");
+    let initial_commit = CommitBuilder::for_new_commit(
+        &missing_user_settings,
+        &store,
+        repo.store().empty_tree_id().clone(),
+    )
+    .write_to_repo(tx.mut_repo());
+    assert_eq!(initial_commit.author().name, "(no name configured)");
+    assert_eq!(initial_commit.author().email, "(no email configured)");
+    assert_eq!(initial_commit.committer().name, "(no name configured)");
+    assert_eq!(initial_commit.committer().email, "(no email configured)");
+
+    let config = config::Config::builder()
+        .set_override("user.name", "Configured User")
+        .unwrap()
+        .set_override("user.email", "configured.user@example.com")
+        .unwrap()
+        .build()
+        .unwrap();
+    let settings = UserSettings::from_config(config);
+    let rewritten_commit = CommitBuilder::for_rewrite_from(&settings, &store, &initial_commit)
+        .write_to_repo(tx.mut_repo());
+
+    assert_eq!(rewritten_commit.author().name, "Configured User");
+    assert_eq!(
+        rewritten_commit.author().email,
+        "configured.user@example.com"
+    );
+    assert_eq!(rewritten_commit.committer().name, "Configured User");
+    assert_eq!(
+        rewritten_commit.committer().email,
+        "configured.user@example.com"
+    );
+}
+
 #[test_case(false ; "local backend")]
 // #[test_case(true ; "git backend")]
 fn test_commit_builder_descendants(use_git: bool) {