show untracked files in jj status

cli/src/cli_util.rs

@@ -385,10 +385,20 @@ impl CommandHelper {
     /// Loads workspace and repo, then snapshots the working copy if allowed.
     #[instrument(skip(self, ui))]
     pub fn workspace_helper(&self, ui: &Ui) -> Result<WorkspaceCommandHelper, CommandError> {
+        Ok(self.workspace_helper_with_stats(ui)?.0)
+    }
+
+    /// Loads workspace and repo, then snapshots the working copy if allowed and
+    /// returns the SnapshotStats.
+    #[instrument(skip(self, ui))]
+    pub fn workspace_helper_with_stats(
+        &self,
+        ui: &Ui,
+    ) -> Result<(WorkspaceCommandHelper, SnapshotStats), CommandError> {
         let mut workspace_command = self.workspace_helper_no_snapshot(ui)?;
 
-        let workspace_command = match workspace_command.maybe_snapshot_impl(ui) {
-            Ok(()) => workspace_command,
+        let (workspace_command, stats) = match workspace_command.maybe_snapshot_impl(ui) {
+            Ok(stats) => (workspace_command, stats),
             Err(SnapshotWorkingCopyError::Command(err)) => return Err(err),
             Err(SnapshotWorkingCopyError::StaleWorkingCopy(err)) => {
                 let auto_update_stale = self.settings().get_bool("snapshot.auto-update-stale")?;
@@ -404,7 +414,7 @@ impl CommandHelper {
             }
         };
 
-        Ok(workspace_command)
+        Ok((workspace_command, stats))
     }
 
     /// Loads workspace and repo, but never snapshots the working copy. Most
@@ -452,7 +462,7 @@ impl CommandHelper {
     pub fn recover_stale_working_copy(
         &self,
         ui: &Ui,
-    ) -> Result<WorkspaceCommandHelper, CommandError> {
+    ) -> Result<(WorkspaceCommandHelper, SnapshotStats), CommandError> {
         let workspace = self.load_workspace()?;
         let op_id = workspace.working_copy().operation_id();
 
@@ -465,7 +475,7 @@ impl CommandHelper {
                 // operation, then merge the divergent operations. The wc_commit_id of the
                 // merged repo wouldn't change because the old one wins, but it's probably
                 // fine if we picked the new wc_commit_id.
-                workspace_command.maybe_snapshot(ui)?;
+                let stats = workspace_command.maybe_snapshot(ui)?;
 
                 let wc_commit_id = workspace_command.get_wc_commit_id().unwrap();
                 let repo = workspace_command.repo().clone();
@@ -516,7 +526,7 @@ impl CommandHelper {
                     }
                 };
 
-                Ok(workspace_command)
+                Ok((workspace_command, stats))
             }
             Err(e @ OpStoreError::ObjectNotFound { .. }) => {
                 writeln!(
@@ -526,8 +536,8 @@ impl CommandHelper {
                 )?;
 
                 let mut workspace_command = self.workspace_helper_no_snapshot(ui)?;
-                workspace_command.create_and_check_out_recovery_commit(ui)?;
-                Ok(workspace_command)
+                let stats = workspace_command.create_and_check_out_recovery_commit(ui)?;
+                Ok((workspace_command, stats))
             }
             Err(e) => Err(e.into()),
         }
@@ -1014,29 +1024,31 @@ impl WorkspaceCommandHelper {
     }
 
     #[instrument(skip_all)]
-    fn maybe_snapshot_impl(&mut self, ui: &Ui) -> Result<(), SnapshotWorkingCopyError> {
-        if self.may_update_working_copy {
-            if self.working_copy_shared_with_git {
-                self.import_git_head(ui).map_err(snapshot_command_error)?;
-            }
-            // Because the Git refs (except HEAD) aren't imported yet, the ref
-            // pointing to the new working-copy commit might not be exported.
-            // In that situation, the ref would be conflicted anyway, so export
-            // failure is okay.
-            self.snapshot_working_copy(ui)?;
+    fn maybe_snapshot_impl(&mut self, ui: &Ui) -> Result<SnapshotStats, SnapshotWorkingCopyError> {
+        if !self.may_update_working_copy {
+            return Ok(SnapshotStats::default());
+        }
 
-            // import_git_refs() can rebase the working-copy commit.
-            if self.working_copy_shared_with_git {
-                self.import_git_refs(ui).map_err(snapshot_command_error)?;
-            }
+        if self.working_copy_shared_with_git {
+            self.import_git_head(ui).map_err(snapshot_command_error)?;
         }
-        Ok(())
+        // Because the Git refs (except HEAD) aren't imported yet, the ref
+        // pointing to the new working-copy commit might not be exported.
+        // In that situation, the ref would be conflicted anyway, so export
+        // failure is okay.
+        let stats = self.snapshot_working_copy(ui)?;
+
+        // import_git_refs() can rebase the working-copy commit.
+        if self.working_copy_shared_with_git {
+            self.import_git_refs(ui).map_err(snapshot_command_error)?;
+        }
+        Ok(stats)
     }
 
     /// Snapshot the working copy if allowed, and import Git refs if the working
     /// copy is collocated with Git.
     #[instrument(skip_all)]
-    pub fn maybe_snapshot(&mut self, ui: &Ui) -> Result<(), CommandError> {
+    pub fn maybe_snapshot(&mut self, ui: &Ui) -> Result<SnapshotStats, CommandError> {
         self.maybe_snapshot_impl(ui)
             .map_err(|err| err.into_command_error())
     }
@@ -1186,7 +1198,10 @@ impl WorkspaceCommandHelper {
         Ok((locked_ws, wc_commit))
     }
 
-    fn create_and_check_out_recovery_commit(&mut self, ui: &Ui) -> Result<(), CommandError> {
+    fn create_and_check_out_recovery_commit(
+        &mut self,
+        ui: &Ui,
+    ) -> Result<SnapshotStats, CommandError> {
         self.check_working_copy_writable()?;
 
         let workspace_id = self.workspace_id().clone();
@@ -1756,7 +1771,10 @@ to the current parents may contain changes from multiple commits.
     }
 
     #[instrument(skip_all)]
-    fn snapshot_working_copy(&mut self, ui: &Ui) -> Result<(), SnapshotWorkingCopyError> {
+    fn snapshot_working_copy(
+        &mut self,
+        ui: &Ui,
+    ) -> Result<SnapshotStats, SnapshotWorkingCopyError> {
         let workspace_id = self.workspace_id().to_owned();
         let get_wc_commit = |repo: &ReadonlyRepo| -> Result<Option<_>, _> {
             repo.view()
@@ -1769,7 +1787,7 @@ to the current parents may contain changes from multiple commits.
         let Some(wc_commit) = get_wc_commit(&repo)? else {
             // If the workspace has been deleted, it's unclear what to do, so we just skip
             // committing the working copy.
-            return Ok(());
+            return Ok(SnapshotStats::default());
         };
         let auto_tracking_matcher = self
             .auto_tracking_matcher(ui)
@@ -1795,8 +1813,8 @@ to the current parents may contain changes from multiple commits.
                     let wc_commit = if let Some(wc_commit) = get_wc_commit(&repo)? {
                         wc_commit
                     } else {
-                        return Ok(()); // The workspace has been deleted (see
-                                       // above)
+                        // The workspace has been deleted (see above)
+                        return Ok(SnapshotStats::default());
                     };
                     (repo, wc_commit)
                 }
@@ -1886,7 +1904,7 @@ See https://jj-vcs.github.io/jj/latest/working-copy/#stale-working-copy \
             .map_err(snapshot_command_error)?;
         print_snapshot_stats(ui, &stats, &self.env.path_converter)
             .map_err(snapshot_command_error)?;
-        Ok(())
+        Ok(stats)
     }
 
     fn update_working_copy(
@@ -2578,35 +2596,48 @@ pub fn print_snapshot_stats(
     stats: &SnapshotStats,
     path_converter: &RepoPathUiConverter,
 ) -> io::Result<()> {
-    // It might make sense to add files excluded by snapshot.auto-track to the
-    // untracked_paths, but they shouldn't be warned every time we do snapshot.
-    // These paths will have to be printed by "jj status" instead.
-    if !stats.untracked_paths.is_empty() {
-        writeln!(ui.warning_default(), "Refused to snapshot some files:")?;
-        let mut formatter = ui.stderr_formatter();
-        for (path, reason) in &stats.untracked_paths {
-            let ui_path = path_converter.format_file_path(path);
-            let message = match reason {
+    // Paths with UntrackedReason::FileNotAutoTracked shouldn't be warned about
+    // every time we make a snapshot. These paths will be printed by
+    // "jj status" instead.
+
+    let mut untracked_paths = stats
+        .untracked_paths
+        .iter()
+        .filter_map(|(path, reason)| {
+            match reason {
                 UntrackedReason::FileTooLarge { size, max_size } => {
                     // Show both exact and human bytes sizes to avoid something
                     // like '1.0MiB, maximum size allowed is ~1.0MiB'
                     let size_approx = HumanByteSize(*size);
                     let max_size_approx = HumanByteSize(*max_size);
-                    format!(
-                        "{size_approx} ({size} bytes); the maximum size allowed is \
-                         {max_size_approx} ({max_size} bytes)",
-                    )
+                    Some((
+                        path,
+                        format!(
+                            "{size_approx} ({size} bytes); the maximum size allowed is \
+                             {max_size_approx} ({max_size} bytes)",
+                        ),
+                    ))
                 }
-            };
+                UntrackedReason::FileNotAutoTracked => None,
+            }
+        })
+        .peekable();
+
+    if untracked_paths.peek().is_some() {
+        writeln!(ui.warning_default(), "Refused to snapshot some files:")?;
+        let mut formatter = ui.stderr_formatter();
+        for (path, message) in untracked_paths {
+            let ui_path = path_converter.format_file_path(path);
             writeln!(formatter, "  {ui_path}: {message}")?;
         }
     }
 
     if let Some(size) = stats
         .untracked_paths
         .values()
-        .map(|reason| match reason {
-            UntrackedReason::FileTooLarge { size, .. } => *size,
+        .filter_map(|reason| match reason {
+            UntrackedReason::FileTooLarge { size, .. } => Some(*size),
+            UntrackedReason::FileNotAutoTracked => None,
         })
         .max()
     {

cli/src/commands/status.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::io;
+
 use itertools::Itertools;
 use jj_lib::copies::CopyRecords;
 use jj_lib::repo::Repo;
@@ -47,7 +49,7 @@ pub(crate) fn cmd_status(
     command: &CommandHelper,
     args: &StatusArgs,
 ) -> Result<(), CommandError> {
-    let workspace_command = command.workspace_helper(ui)?;
+    let (workspace_command, snapshot_stats) = command.workspace_helper_with_stats(ui)?;
     let repo = workspace_command.repo();
     let maybe_wc_commit = workspace_command
         .get_wc_commit_id()
@@ -63,26 +65,44 @@ pub(crate) fn cmd_status(
     if let Some(wc_commit) = &maybe_wc_commit {
         let parent_tree = wc_commit.parent_tree(repo.as_ref())?;
         let tree = wc_commit.tree()?;
-        if tree.id() == parent_tree.id() {
+
+        let wc_has_changes = tree.id() != parent_tree.id();
+        let wc_has_untracked = !snapshot_stats.untracked_paths.is_empty();
+        if !wc_has_changes && !wc_has_untracked {
             writeln!(formatter, "The working copy is clean")?;
         } else {
-            writeln!(formatter, "Working copy changes:")?;
-            let mut copy_records = CopyRecords::default();
-            for parent in wc_commit.parent_ids() {
-                let records = get_copy_records(repo.store(), parent, wc_commit.id(), &matcher)?;
-                copy_records.add_records(records)?;
+            if wc_has_changes {
+                writeln!(formatter, "Working copy changes:")?;
+                let mut copy_records = CopyRecords::default();
+                for parent in wc_commit.parent_ids() {
+                    let records = get_copy_records(repo.store(), parent, wc_commit.id(), &matcher)?;
+                    copy_records.add_records(records)?;
+                }
+                let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]);
+                let width = ui.term_width();
+                diff_renderer.show_diff(
+                    ui,
+                    formatter,
+                    &parent_tree,
+                    &tree,
+                    &matcher,
+                    &copy_records,
+                    width,
+                )?;
+            }
+
+            if wc_has_untracked {
+                // TODO: make sure this always display all untracked non-ignored files, even
+                // when using watchman. See https://github.com/jj-vcs/jj/commit/168c7979feab40d58f49fe19683975697a7bc089 for details.
+                writeln!(formatter, "Untracked paths:")?;
+                formatter.with_label("diff", |formatter| {
+                    for path in snapshot_stats.untracked_paths.keys() {
+                        let ui_path = workspace_command.path_converter().format_file_path(path);
+                        writeln!(formatter.labeled("untracked"), "? {ui_path}")?;
+                    }
+                    io::Result::Ok(())
+                })?;
             }
-            let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]);
-            let width = ui.term_width();
-            diff_renderer.show_diff(
-                ui,
-                formatter,
-                &parent_tree,
-                &tree,
-                &matcher,
-                &copy_records,
-                width,
-            )?;
         }
 
         // TODO: Conflicts should also be filtered by the `matcher`. See the related

cli/src/config/colors.toml

@@ -93,6 +93,7 @@
 "diff added" = { fg = "green" }
 "diff token" = { underline = true }
 "diff modified" = "cyan"
+"diff untracked" = "magenta"
 "diff renamed" = "cyan"
 "diff copied" = "green"
 "diff access-denied" = { bg = "red" }

cli/tests/test_status_command.rs

@@ -368,3 +368,69 @@ fn test_status_simplify_conflict_sides() {
     Then run `jj squash` to move the resolution into the conflicted commit.
     "###);
 }
+
+#[test]
+fn test_status_untracked_files() {
+    let test_env = TestEnvironment::default();
+    test_env.add_config(r#"snapshot.auto-track = "none()""#);
+
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    std::fs::write(repo_path.join("initially-untracked-file"), "...").unwrap();
+    std::fs::write(repo_path.join("always-untracked-file"), "...").unwrap();
+
+    let stdout = test_env.jj_cmd_success(&repo_path, &["status"]);
+    insta::assert_snapshot!(stdout, @r"
+    Untracked paths:
+    ? always-untracked-file
+    ? initially-untracked-file
+    Working copy : qpvuntsm 230dd059 (empty) (no description set)
+    Parent commit: zzzzzzzz 00000000 (empty) (no description set)
+    ");
+
+    test_env.jj_cmd_success(&repo_path, &["file", "track", "initially-untracked-file"]);
+
+    let stdout = test_env.jj_cmd_success(&repo_path, &["status"]);
+    insta::assert_snapshot!(stdout, @r"
+    Working copy changes:
+    A initially-untracked-file
+    Untracked paths:
+    ? always-untracked-file
+    Working copy : qpvuntsm 203bfea9 (no description set)
+    Parent commit: zzzzzzzz 00000000 (empty) (no description set)
+    ");
+
+    test_env.jj_cmd_ok(&repo_path, &["new"]);
+
+    let stdout = test_env.jj_cmd_success(&repo_path, &["status"]);
+    insta::assert_snapshot!(stdout, @r"
+    Untracked paths:
+    ? always-untracked-file
+    Working copy : mzvwutvl 69b48d55 (empty) (no description set)
+    Parent commit: qpvuntsm 203bfea9 (no description set)
+    ");
+
+    test_env.jj_cmd_success(&repo_path, &["file", "untrack", "initially-untracked-file"]);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["status"]);
+    insta::assert_snapshot!(stdout, @r"
+    Working copy changes:
+    D initially-untracked-file
+    Untracked paths:
+    ? always-untracked-file
+    ? initially-untracked-file
+    Working copy : mzvwutvl 16169825 (no description set)
+    Parent commit: qpvuntsm 203bfea9 (no description set)
+    ");
+
+    test_env.jj_cmd_ok(&repo_path, &["new"]);
+
+    let stdout = test_env.jj_cmd_success(&repo_path, &["status"]);
+    insta::assert_snapshot!(stdout, @r"
+    Untracked paths:
+    ? always-untracked-file
+    ? initially-untracked-file
+    Working copy : yostqsxw 9b87b665 (empty) (no description set)
+    Parent commit: mzvwutvl 16169825 (no description set)
+    ");
+}