cli: add reset-author-on-edit revset

It's common to create empty working-copy commits while using jj, and
currently the author timestamp for a commit is only set when it is first
created. If you create an empty commit, then don't work on a repo for a
few days, and then start working on a new feature without abandoning the
working-copy commit, the author timestamp will remain as the time the
commit was created rather than being updated to the time that work began
or finished.

This commit adds a new revset `reset-author-on-edit` which specifies
which commits should have their author information updated whenever
changes are made to them. This can be configured to fit many different
workflows. By default, empty commits with no description by the current
user have their author timestamps reset, meaning that the author
timestamp will become finalized whenever a commit is given a description
or becomes non-empty. Rebasing commits never updates the author
timestamp.

CHANGELOG.md

@@ -66,6 +66,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * `jj prev` and `jj next` have gained a `--conflict` flag which moves you
   to the next conflict in a child commit.
 
+* `revsets.reset-author-on-edit` can be used to automatically reset author
+  information/timestamp when rewriting commits. Default behavior is to reset
+  author timestamp when rewriting empty commits with no description if authored
+  by current user.
+
 ### Fixed bugs
 
 ## [0.18.0] - 2024-06-05

cli/src/cli_util.rs

@@ -400,6 +400,30 @@ impl ReadonlyUserRepo {
     pub fn git_backend(&self) -> Option<&GitBackend> {
         self.repo.store().backend_impl().downcast_ref()
     }
+
+    pub fn id_prefix_context(
+        &self,
+        settings: &UserSettings,
+        revset_extensions: &Arc<RevsetExtensions>,
+        revset_parse_context: &RevsetParseContext,
+    ) -> Result<&IdPrefixContext, CommandError> {
+        self.id_prefix_context.get_or_try_init(|| {
+            let mut context: IdPrefixContext = IdPrefixContext::new(revset_extensions.clone());
+            let revset_string: String = settings
+                .config()
+                .get_string("revsets.short-prefixes")
+                .unwrap_or_else(|_| settings.default_revset());
+            if !revset_string.is_empty() {
+                let (expression, modifier) =
+                    revset::parse_with_modifier(&revset_string, revset_parse_context).map_err(
+                        |err| config_error_with_message("Invalid `revsets.short-prefixes`", err),
+                    )?;
+                let (None | Some(RevsetModifier::All)) = modifier;
+                context = context.disambiguate_within(revset::optimize(expression));
+            }
+            Ok(context)
+        })
+    }
 }
 
 /// A branch that should be advanced to satisfy the "advance-branches" feature.
@@ -1001,24 +1025,11 @@ impl WorkspaceCommandHelper {
     }
 
     pub fn id_prefix_context(&self) -> Result<&IdPrefixContext, CommandError> {
-        self.user_repo.id_prefix_context.get_or_try_init(|| {
-            let mut context: IdPrefixContext = IdPrefixContext::new(self.revset_extensions.clone());
-            let revset_string: String = self
-                .settings
-                .config()
-                .get_string("revsets.short-prefixes")
-                .unwrap_or_else(|_| self.settings.default_revset());
-            if !revset_string.is_empty() {
-                let (expression, modifier) =
-                    revset::parse_with_modifier(&revset_string, &self.revset_parse_context())
-                        .map_err(|err| {
-                            config_error_with_message("Invalid `revsets.short-prefixes`", err)
-                        })?;
-                let (None | Some(RevsetModifier::All)) = modifier;
-                context = context.disambiguate_within(revset::optimize(expression));
-            }
-            Ok(context)
-        })
+        self.user_repo.id_prefix_context(
+            &self.settings,
+            &self.revset_extensions,
+            &self.revset_parse_context(),
+        )
     }
 
     pub fn template_aliases_map(&self) -> &TemplateAliasesMap {
@@ -1229,14 +1240,28 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
         })?;
         drop(progress);
         if new_tree_id != *wc_commit.tree_id() {
+            let reset_author = check_reset_author_on_edit(
+                &self.settings,
+                &workspace_id,
+                &self.user_repo,
+                &self.revset_extensions,
+                &self.revset_aliases_map,
+                &self.path_converter,
+                self.user_repo.repo.as_ref(),
+                wc_commit.id(),
+            )?;
             let mut tx =
                 start_repo_transaction(&self.user_repo.repo, &self.settings, &self.string_args);
             tx.set_is_snapshot(true);
             let mut_repo = tx.mut_repo();
-            let commit = mut_repo
+            let mut commit_builder = mut_repo
                 .rewrite_commit(&self.settings, &wc_commit)
-                .set_tree_id(new_tree_id)
-                .write()?;
+                .set_tree_id(new_tree_id);
+            if reset_author {
+                let committer = commit_builder.committer().clone();
+                commit_builder = commit_builder.set_author(committer);
+            }
+            let commit = commit_builder.write()?;
             mut_repo.set_wc_commit(workspace_id, commit.id().clone())?;
 
             // Rebase descendants
@@ -1585,6 +1610,50 @@ Then run `jj squash` to move the resolution into the conflicted commit."#,
     }
 }
 
+#[allow(clippy::too_many_arguments)]
+fn check_reset_author_on_edit(
+    settings: &UserSettings,
+    workspace_id: &WorkspaceId,
+    // Required to build IdPrefixContext
+    user_repo: &ReadonlyUserRepo,
+    revset_extensions: &Arc<RevsetExtensions>,
+    revset_aliases_map: &RevsetAliasesMap,
+    path_converter: &RepoPathUiConverter,
+    // May be different from user_repo for MutableRepo
+    repo: &dyn Repo,
+    commit_id: &CommitId,
+) -> Result<bool, CommandError> {
+    let workspace_context = RevsetWorkspaceContext {
+        path_converter,
+        workspace_id,
+    };
+    let revset_parse_context = RevsetParseContext::new(
+        revset_aliases_map,
+        settings.user_email(),
+        revset_extensions,
+        Some(workspace_context),
+    );
+    let id_prefix_context =
+        user_repo.id_prefix_context(settings, revset_extensions, &revset_parse_context)?;
+    let reset_author_on_edit = settings
+        .config()
+        .get_string("revsets.reset-author-on-edit")?;
+    let reset_author_revset = revset::parse(&reset_author_on_edit, &revset_parse_context)
+        .map_err(|e| config_error_with_message("Invalid `revsets.reset-author-on-edit`", e))?;
+    let mut expression = RevsetExpressionEvaluator::new(
+        repo,
+        revset_extensions.clone(),
+        id_prefix_context,
+        reset_author_revset,
+    );
+    expression.intersect_with(&RevsetExpression::commit(commit_id.clone()));
+    Ok(expression
+        .evaluate_to_commit_ids()
+        .map_err(|e| config_error_with_message("Invalid `revsets.reset-author-on-edit`", e))?
+        .next()
+        .is_some())
+}
+
 /// A [`Transaction`] tied to a particular workspace.
 /// `WorkspaceCommandTransaction`s are created with
 /// [`WorkspaceCommandHelper::start_transaction`] and committed with
@@ -1631,7 +1700,24 @@ impl WorkspaceCommandTransaction<'_> {
         commit: &Commit,
     ) -> Result<CommitBuilder, CommandError> {
         let settings = &self.helper.settings;
-        Ok(self.tx.mut_repo().rewrite_commit(settings, commit))
+        let reset_author = check_reset_author_on_edit(
+            settings,
+            self.helper.workspace_id(),
+            &self.helper.user_repo,
+            &self.helper.revset_extensions,
+            &self.helper.revset_aliases_map,
+            &self.helper.path_converter,
+            self.tx.mut_repo(),
+            commit.id(),
+        )?;
+
+        let commit_builder = self.tx.mut_repo().rewrite_commit(settings, commit);
+        if reset_author {
+            let committer = commit_builder.committer().clone();
+            Ok(commit_builder.set_author(committer))
+        } else {
+            Ok(commit_builder)
+        }
     }
 
     pub fn format_commit_summary(&self, commit: &Commit) -> String {

cli/src/commands/untrack.rs

@@ -58,6 +58,8 @@ pub(crate) fn cmd_untrack(
         tree_builder.set_or_remove(path, Merge::absent());
     }
     let new_tree_id = tree_builder.write_tree(&store)?;
+    // Cannot use `rewrite_edited_commit` since workspace is locked, so author
+    // information will not be reset automatically.
     let new_commit = tx
         .mut_repo()
         .rewrite_commit(command.settings(), &wc_commit)

cli/src/config-schema.json

@@ -370,6 +370,11 @@
                     "type": "string",
                     "description": "Revisions to give shorter change and commit IDs to",
                     "default": "<revsets.log>"
+                },
+                "reset-author-on-edit": {
+                    "type": "string",
+                    "description": "Revisions which should have their author information and timestamp reset when they are edited",
+                    "default": "mine() & empty() & description(exact:\"\")"
                 }
             },
             "additionalProperties": {

cli/src/config/revsets.toml

@@ -4,6 +4,8 @@
 [revsets]
 fix = "reachable(@, mutable())"
 log = "@ | ancestors(immutable_heads().., 2) | trunk()"
+reset-author-on-edit = "none()" # To make tests pass in this commit
+# reset-author-on-edit = 'mine() & empty() & description(exact:"")'
 
 [revset-aliases]
 'trunk()' = '''

cli/tests/runner.rs

@@ -51,6 +51,7 @@ mod test_operations;
 mod test_parallelize_command;
 mod test_rebase_command;
 mod test_repo_change_report;
+mod test_reset_author_on_edit;
 mod test_resolve_command;
 mod test_restore_command;
 mod test_revset_output;

cli/tests/test_reset_author_on_edit.rs

@@ -0,0 +1,146 @@
+// Copyright 2024 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.
+
+use std::path::Path;
+
+use crate::common::TestEnvironment;
+
+#[test]
+fn test_reset_author_default_config() {
+    let test_env = TestEnvironment::default();
+    test_env
+        .add_config(r#"revsets.reset-author-on-edit = 'mine() & empty() & description(exact:"")'"#);
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    test_env.jj_cmd_ok(&repo_path, &["new", "root()"]);
+    std::fs::write(repo_path.join("file1"), "a").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "1"]);
+
+    test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-m", "2"]);
+    std::fs::write(repo_path.join("file2"), "b").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["st"]); // Snapshot before user gets changed
+
+    test_env.jj_cmd_ok(
+        &repo_path,
+        &[
+            "new",
+            "--config-toml",
+            "user.email='[email protected]'",
+            "root()",
+            "-m",
+            "3",
+        ],
+    );
+    std::fs::write(repo_path.join("file3"), "c").unwrap();
+
+    insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
+    @  cc20a19b417d [email protected] 2001-02-03 04:05:12.000 +07:00 [email protected] 2001-02-03 04:05:13.000 +07:00 3
+    │ ◉  83cbfda1cbc7 [email protected] 2001-02-03 04:05:10.000 +07:00 [email protected] 2001-02-03 04:05:11.000 +07:00 2
+    ├─╯
+    │ ◉  b1623baa27e4 [email protected] 2001-02-03 04:05:09.000 +07:00 [email protected] 2001-02-03 04:05:09.000 +07:00 1
+    ├─╯
+    ◉  000000000000 1970-01-01 00:00:00.000 +00:00 1970-01-01 00:00:00.000 +00:00
+    "###);
+
+    test_env.jj_cmd_ok(&repo_path, &["edit", "description(1)"]);
+    std::fs::write(repo_path.join("file4"), "d").unwrap();
+
+    insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
+    @  de11fa19c3a7 [email protected] 2001-02-03 04:05:09.000 +07:00 [email protected] 2001-02-03 04:05:15.000 +07:00 1
+    │ ◉  cc20a19b417d [email protected] 2001-02-03 04:05:12.000 +07:00 [email protected] 2001-02-03 04:05:13.000 +07:00 3
+    ├─╯
+    │ ◉  83cbfda1cbc7 [email protected] 2001-02-03 04:05:10.000 +07:00 [email protected] 2001-02-03 04:05:11.000 +07:00 2
+    ├─╯
+    ◉  000000000000 1970-01-01 00:00:00.000 +00:00 1970-01-01 00:00:00.000 +00:00
+    "###);
+}
+
+#[test]
+fn test_reset_author_timestamp() {
+    let test_env = TestEnvironment::default();
+    test_env.add_config(r#"revsets.reset-author-on-edit = "description(1)""#);
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-m", "1"]);
+    std::fs::write(repo_path.join("file1"), "a").unwrap();
+
+    test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-m", "2"]);
+    std::fs::write(repo_path.join("file2"), "b").unwrap();
+
+    insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
+    @  0c939f685bff [email protected] 2001-02-03 04:05:09.000 +07:00 [email protected] 2001-02-03 04:05:10.000 +07:00 2
+    │ ◉  b1623baa27e4 [email protected] 2001-02-03 04:05:09.000 +07:00 [email protected] 2001-02-03 04:05:09.000 +07:00 1
+    ├─╯
+    ◉  000000000000 1970-01-01 00:00:00.000 +00:00 1970-01-01 00:00:00.000 +00:00
+    "###);
+}
+
+#[test]
+fn test_reset_author_information() {
+    let test_env = TestEnvironment::default();
+    test_env.add_config(r#"revsets.reset-author-on-edit = "description(1)""#);
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    test_env.jj_cmd_ok(
+        &repo_path,
+        &[
+            "new",
+            "--config-toml",
+            "user.email='[email protected]'",
+            "root()",
+            "-m",
+            "1",
+        ],
+    );
+    std::fs::write(repo_path.join("file1"), "a").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["st"]); // Snapshot using a different user
+
+    test_env.jj_cmd_ok(
+        &repo_path,
+        &[
+            "new",
+            "--config-toml",
+            "user.email='[email protected]'",
+            "root()",
+            "-m",
+            "2",
+        ],
+    );
+    std::fs::write(repo_path.join("file2"), "b").unwrap();
+
+    insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
+    @  53407752d5a2 [email protected] 2001-02-03 04:05:10.000 +07:00 [email protected] 2001-02-03 04:05:11.000 +07:00 2
+    │ ◉  b1623baa27e4 [email protected] 2001-02-03 04:05:09.000 +07:00 [email protected] 2001-02-03 04:05:09.000 +07:00 1
+    ├─╯
+    ◉  000000000000 1970-01-01 00:00:00.000 +00:00 1970-01-01 00:00:00.000 +00:00
+    "###);
+}
+
+fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
+    let template = r#"
+    separate(
+        " ",
+        commit_id.short(),
+        author.email(),
+        author.timestamp(),
+        committer.email(),
+        committer.timestamp(),
+        description,
+    )
+    "#;
+    test_env.jj_cmd_success(cwd, &["log", "-T", template])
+}

docs/config.md

@@ -214,6 +214,31 @@ revset-aliases."immutable_heads()" = "main@origin | (main@origin.. & ~mine())"
 Ancestors of the configured set are also immutable. The root commit is always
 immutable even if the set is empty.
 
+### Author timestamp behavior
+
+Since `jj` often creates empty working-copy commits automatically, it can be
+undesirable to have the author timestamp be set at the time a commit is created,
+since you might not actually make any changes in that working-copy commit until
+later. To resolve this problem, if you make changes to an empty commit with no
+description that you authored, `jj` will automatically reset the author
+timestamp for that commit. This means that by default, the author timestamp
+represents the time that the first changes were made in a commit.
+
+This behavior can be configured using `revsets.reset-author-on-edit`, which has
+a default value of `mine() & empty() & description(exact:"")`. Any commits in
+this revset will have their author information and timestamp reset when they are
+rewritten, except for while rebasing. For example, to reset the author timestamp
+when making changes to any commit that you authored:
+
+```toml
+# Reset author when editing any commit authored by current user
+revsets.reset-author-on-edit = "mine()"
+```
+
+A more Git-like behavior might be to finalize a commit's author timestamp when a
+description is set (e.g. with `jj commit`), which could be achieved using
+`mine() & description(exact:"")`.
+
 ## Log
 
 ### Default revisions