cli: clarify error message when jj untrack argument is not ignored

As pointed out by @arxanas in #88, the message saying something like
"At least 'bin/.DS_Store' was added back ..." is confusing especially
when the command you ran was just `jj untrack bin/.DS_Store`. Let's
clarify the message by saying exactly how many more files there are,
and specialize the message for when there is only one file. Also
update the message to say "would be added back" instead of "was added
back" since we don't actually change anything if some files would be
added back (since 4b91ad408c).

Should we even list all the files? I'm concerned that such a list
could be very long. On the other hand, it can also be annoying to have
to run `jj untrack some/dir/` and only be told about single file to
add to the ignore patterns every time.

src/commands.rs

@@ -1820,14 +1820,25 @@ fn cmd_untrack(
     let wc_tree_id = locked_working_copy.write_tree();
     if wc_tree_id != new_tree_id {
         let wc_tree = store.get_tree(&RepoPath::root(), &wc_tree_id)?;
-        if let Some((path, _value)) = wc_tree.entries_matching(matcher.as_ref()).next() {
+        let added_back = wc_tree.entries_matching(matcher.as_ref()).collect_vec();
+        if !added_back.is_empty() {
             locked_working_copy.discard();
-            let ui_path = ui.format_file_path(workspace_command.workspace_root(), &path);
-            return Err(CommandError::UserError(format!(
-                "At least '{}' was added back because it is not ignored. Make sure it's ignored, \
-                 then try again.",
-                ui_path
-            )));
+            let path = &added_back[0].0;
+            let ui_path = ui.format_file_path(workspace_command.workspace_root(), path);
+            if added_back.len() > 1 {
+                return Err(CommandError::UserError(format!(
+                    "'{}' and {} other files would be added back because they're not ignored. \
+                     Make sure they're ignored, then try again.",
+                    ui_path,
+                    added_back.len() - 1
+                )));
+            } else {
+                return Err(CommandError::UserError(format!(
+                    "'{}' would be added back because it's not ignored. Make sure it's ignored, \
+                     then try again.",
+                    ui_path
+                )));
+            }
         } else {
             // This means there were some concurrent changes made in the working copy. We
             // don't want to mix those in, so reset the working copy again.

src/ui.rs

@@ -105,6 +105,7 @@ impl<'stdout> Ui<'stdout> {
     }
 
     pub fn write_error(&mut self, text: &str) -> io::Result<()> {
+        // TODO: We should print the error to stderr
         let mut formatter = self.stdout_formatter();
         formatter.add_label(String::from("error"))?;
         formatter.write_str(text)?;

tests/test_untrack_command.rs

@@ -0,0 +1,98 @@
+// Copyright 2022 Google LLC
+//
+// 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::PathBuf;
+
+use jujutsu::testutils::{get_stdout_string, TestEnvironment};
+
+#[test]
+fn test_untrack() {
+    let test_env = TestEnvironment::default();
+    test_env
+        .jj_cmd(test_env.env_root(), &["init", "repo"])
+        .assert()
+        .success();
+    let repo_path = test_env.env_root().join("repo");
+
+    std::fs::write(repo_path.join("file1"), "initial").unwrap();
+    std::fs::write(repo_path.join("file1.bak"), "initial").unwrap();
+    std::fs::write(repo_path.join("file2.bak"), "initial").unwrap();
+    let target_dir = repo_path.join("target");
+    std::fs::create_dir(&target_dir).unwrap();
+    std::fs::write(target_dir.join("file2"), "initial").unwrap();
+    std::fs::write(target_dir.join("file3"), "initial").unwrap();
+
+    // Run a command so all the files get tracked, then add "*.bak" to the ignore
+    // patterns
+    test_env.jj_cmd(&repo_path, &["st"]).assert().success();
+    std::fs::write(repo_path.join(".gitignore"), "*.bak\n").unwrap();
+    let files_before =
+        get_stdout_string(&test_env.jj_cmd(&repo_path, &["files"]).assert().success());
+
+    // Errors out when a specified file is not ignored
+    let assert = test_env
+        .jj_cmd(&repo_path, &["untrack", "file1", "file1.bak"])
+        .assert()
+        .failure();
+    assert.stdout(
+        "Error: 'file1' would be added back because it's not ignored. Make sure it's ignored, \
+         then try again.\n",
+    );
+    let files_after =
+        get_stdout_string(&test_env.jj_cmd(&repo_path, &["files"]).assert().success());
+    // There should be no changes to the state when there was an error
+    assert_eq!(files_after, files_before);
+
+    // Can untrack a single file
+    assert!(files_before.contains("file1.bak\n"));
+    test_env
+        .jj_cmd(&repo_path, &["untrack", "file1.bak"])
+        .assert()
+        .success()
+        .stdout("");
+    let files_after =
+        get_stdout_string(&test_env.jj_cmd(&repo_path, &["files"]).assert().success());
+    // The file is no longer tracked
+    assert!(!files_after.contains("file1.bak"));
+    // Other files that match the ignore pattern are not untracked
+    assert!(files_after.contains("file2.bak"));
+    // The files still exist on disk
+    assert!(repo_path.join("file1.bak").exists());
+    assert!(repo_path.join("file2.bak").exists());
+
+    // Errors out when multiple specified files are not ignored
+    let assert = test_env
+        .jj_cmd(&repo_path, &["untrack", "target"])
+        .assert()
+        .failure();
+    assert_eq!(
+        get_stdout_string(&assert),
+        format!(
+            "Error: '{}' and 1 other files would be added back because they're not ignored. Make \
+             sure they're ignored, then try again.\n",
+            PathBuf::from("target").join("file2").display()
+        )
+    );
+
+    // Can untrack after adding to ignore patterns
+    std::fs::write(repo_path.join(".gitignore"), ".bak\ntarget/\n").unwrap();
+    test_env
+        .jj_cmd(&repo_path, &["untrack", "target"])
+        .assert()
+        .success()
+        .stdout("");
+    let files_after =
+        get_stdout_string(&test_env.jj_cmd(&repo_path, &["files"]).assert().success());
+    assert!(!files_after.contains("target"));
+}