cli: fix: add --include-unchanged-files flag to allow fixing as yet u…

…nchanged files

This enables workflows like "insert a commit that reformats the code in one of
my project directories".

`jj fix --include-unchanged-files` is an easy way to fix everything in the repo.

`jj fix --include-unchanged-files <file...>` fixes all of the `<files>` even if they are
unchanged.

This is mostly orthogonal to other features, so not many tests are added.

This is a significant and simple enough improvement that I think it's
appropriate to make it here instead of waiting for a `jj run`-based solution.

CHANGELOG.md

@@ -20,6 +20,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### New features
 
+* `jj fix` now allows fixing unchanged files with the `--include-unchanged-files` flag. This
+  can be used to more easily introduce automatic formatting changes in a new
+  commit separate from other changes.
+
 ### Fixed bugs
 
  * Fixed panic when parsing invalid conflict markers of a particular form.

cli/src/commands/fix.rs

@@ -28,6 +28,7 @@ use jj_lib::fileset;
 use jj_lib::fileset::FilesetExpression;
 use jj_lib::matchers::EverythingMatcher;
 use jj_lib::matchers::Matcher;
+use jj_lib::merged_tree::MergedTree;
 use jj_lib::merged_tree::MergedTreeBuilder;
 use jj_lib::merged_tree::TreeDiffEntry;
 use jj_lib::repo::Repo;
@@ -36,6 +37,7 @@ use jj_lib::repo_path::RepoPathUiConverter;
 use jj_lib::revset::RevsetExpression;
 use jj_lib::revset::RevsetIteratorExt;
 use jj_lib::store::Store;
+use jj_lib::tree::Tree;
 use pollster::FutureExt;
 use rayon::iter::IntoParallelIterator;
 use rayon::prelude::ParallelIterator;
@@ -125,6 +127,10 @@ pub(crate) struct FixArgs {
     /// Fix only these paths
     #[arg(value_hint = clap::ValueHint::AnyPath)]
     paths: Vec<String>,
+    /// Fix unchanged files in addition to changed ones. If no paths are
+    /// specified, all files in the repo will be fixed.
+    #[arg(long)]
+    include_unchanged_files: bool,
 }
 
 #[instrument(skip_all)]
@@ -177,19 +183,22 @@ pub(crate) fn cmd_fix(
     for commit in commits.iter().rev() {
         let mut paths: HashSet<RepoPathBuf> = HashSet::new();
 
-        // Fix all paths that were fixed in ancestors, so we don't lose those changes.
-        // We do this instead of rebasing onto those changes, to avoid merge conflicts.
-        for parent_id in commit.parent_ids() {
-            if let Some(parent_paths) = commit_paths.get(parent_id) {
-                paths.extend(parent_paths.iter().cloned());
+        // If --include-unchanged-files, we always fix every matching file in the tree.
+        // Otherwise, we fix the matching changed files in this commit, plus any that
+        // were fixed in ancestors, so we don't lose those changes. We do this
+        // instead of rebasing onto those changes, to avoid merge conflicts.
+        let parent_tree = if args.include_unchanged_files {
+            MergedTree::resolved(Tree::empty(tx.repo().store().clone(), RepoPathBuf::root()))
+        } else {
+            for parent_id in commit.parent_ids() {
+                if let Some(parent_paths) = commit_paths.get(parent_id) {
+                    paths.extend(parent_paths.iter().cloned());
+                }
             }
-        }
-
-        // Also fix any new paths that were changed in this commit.
-        let tree = commit.tree()?;
-        let parent_tree = commit.parent_tree(tx.repo())?;
+            commit.parent_tree(tx.repo())?
+        };
         // TODO: handle copy tracking
-        let mut diff_stream = parent_tree.diff_stream(&tree, &matcher);
+        let mut diff_stream = parent_tree.diff_stream(&commit.tree()?, &matcher);
         async {
             while let Some(TreeDiffEntry {
                 path: repo_path,

cli/tests/cli-reference@.md.snap

@@ -900,6 +900,7 @@ will be removed in a future version.
 ###### **Options:**
 
 * `-s`, `--source <SOURCE>` — Fix files in the specified revision(s) and their descendants. If no revisions are specified, this defaults to the `revsets.fix` setting, or `reachable(@, mutable())` if it is not set
+* `--include-unchanged-files` — Fix unchanged files in addition to changed ones. If no paths are specified, all files in the repo will be fixed
 
 
 

cli/tests/test_fix_command.rs

@@ -1123,3 +1123,150 @@ fn test_fix_resolve_conflict() {
     CONTENT
     "###);
 }
+
+#[test]
+fn test_all_files() {
+    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 formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter");
+    assert!(formatter_path.is_file());
+    let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\");
+
+    // Consider a few cases:
+    // File A:     in patterns,     changed in child
+    // File B:     in patterns, NOT changed in child
+    // File C: NOT in patterns, NOT changed in child
+    // File D: NOT in patterns,     changed in child
+    // Some files will be in subdirectories to make sure we're covering that aspect
+    // of matching.
+    test_env.add_config(&format!(
+        r###"
+        [fix.tools.tool]
+        command = ["{formatter}", "--append", "fixed"]
+        patterns = ["a/a", "b/b"]
+        "###,
+        formatter = escaped_formatter_path.as_str()
+    ));
+
+    std::fs::create_dir(repo_path.join("a")).unwrap();
+    std::fs::create_dir(repo_path.join("b")).unwrap();
+    std::fs::create_dir(repo_path.join("c")).unwrap();
+    std::fs::write(repo_path.join("a/a"), "parent aaa\n").unwrap();
+    std::fs::write(repo_path.join("b/b"), "parent bbb\n").unwrap();
+    std::fs::write(repo_path.join("c/c"), "parent ccc\n").unwrap();
+    std::fs::write(repo_path.join("ddd"), "parent ddd\n").unwrap();
+    let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "parent"]);
+
+    std::fs::write(repo_path.join("a/a"), "child aaa\n").unwrap();
+    std::fs::write(repo_path.join("ddd"), "child ddd\n").unwrap();
+    let (_stdout, _stderr) = test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "child"]);
+
+    // Specifying files means exactly those files will be fixed in each revision,
+    // although some like file C won't have any tools configured to make changes to
+    // them. Specified but unfixed files are silently skipped, whether they lack
+    // configuration, are ignored, don't exist, aren't normal files, etc.
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &[
+            "fix",
+            "--include-unchanged-files",
+            "b/b",
+            "c/c",
+            "does_not.exist",
+        ],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Fixed 2 commits of 2 checked.
+    Working copy now at: rlvkpnrz c098d165 child
+    Parent commit      : qpvuntsm 0bb31627 parent
+    Added 0 files, modified 1 files, removed 0 files
+    "###);
+
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "a/a", "-r", "@-"]);
+    insta::assert_snapshot!(content, @r###"
+    parent aaa
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "b/b", "-r", "@-"]);
+    insta::assert_snapshot!(content, @r###"
+    parent bbb
+    fixed
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "c/c", "-r", "@-"]);
+    insta::assert_snapshot!(content, @r###"
+    parent ccc
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "ddd", "-r", "@-"]);
+    insta::assert_snapshot!(content, @r###"
+    parent ddd
+    "###);
+
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "a/a", "-r", "@"]);
+    insta::assert_snapshot!(content, @r###"
+    child aaa
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "b/b", "-r", "@"]);
+    insta::assert_snapshot!(content, @r###"
+    parent bbb
+    fixed
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "c/c", "-r", "@"]);
+    insta::assert_snapshot!(content, @r###"
+    parent ccc
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "ddd", "-r", "@"]);
+    insta::assert_snapshot!(content, @r###"
+    child ddd
+    "###);
+
+    // Not specifying files means all files will be fixed in each revision.
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "--include-unchanged-files"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Fixed 2 commits of 2 checked.
+    Working copy now at: rlvkpnrz c5d0aa1d child
+    Parent commit      : qpvuntsm b4d02ca9 parent
+    Added 0 files, modified 2 files, removed 0 files
+    "###);
+
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "a/a", "-r", "@-"]);
+    insta::assert_snapshot!(content, @r###"
+    parent aaa
+    fixed
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "b/b", "-r", "@-"]);
+    insta::assert_snapshot!(content, @r###"
+    parent bbb
+    fixed
+    fixed
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "c/c", "-r", "@-"]);
+    insta::assert_snapshot!(content, @r###"
+    parent ccc
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "ddd", "-r", "@-"]);
+    insta::assert_snapshot!(content, @r###"
+    parent ddd
+    "###);
+
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "a/a", "-r", "@"]);
+    insta::assert_snapshot!(content, @r###"
+    child aaa
+    fixed
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "b/b", "-r", "@"]);
+    insta::assert_snapshot!(content, @r###"
+    parent bbb
+    fixed
+    fixed
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "c/c", "-r", "@"]);
+    insta::assert_snapshot!(content, @r###"
+    parent ccc
+    "###);
+    let content = test_env.jj_cmd_success(&repo_path, &["file", "show", "ddd", "-r", "@"]);
+    insta::assert_snapshot!(content, @r###"
+    child ddd
+    "###);
+}