sparse: parse and print patterns as workspace-relative paths

Per comment in
https://github.com/martinvonz/jj/pull/3379#pullrequestreview-1963841604

Though cwd-relative path might be useful for --remove, I don't think that's
the common use case. The new behavior is consistent with --edit.

CHANGELOG.md

@@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * The `git_head` template keyword now returns an optional value instead of a
   list of 0 or 1 element.
 
+* The `jj sparse` subcommands now parse and print patterns as workspace-relative
+  paths.
+
 ### New features
 
 * Config now supports rgb hex colors (in the form `#rrggbb`) wherever existing color names are supported.

cli/src/commands/sparse.rs

@@ -53,11 +53,20 @@ pub(crate) struct SparseListArgs {}
 #[derive(clap::Args, Clone, Debug)]
 pub(crate) struct SparseSetArgs {
     /// Patterns to add to the working copy
-    #[arg(long, value_hint = clap::ValueHint::AnyPath)]
+    #[arg(
-    add: Vec<String>,
+        long,
+        value_hint = clap::ValueHint::AnyPath,
+        value_parser = |s: &str| RepoPathBuf::from_relative_path(s),
+    )]
+    add: Vec<RepoPathBuf>,
     /// Patterns to remove from the working copy
-    #[arg(long, conflicts_with = "clear", value_hint = clap::ValueHint::AnyPath)]
+    #[arg(
-    remove: Vec<String>,
+        long,
+        conflicts_with = "clear",
+        value_hint = clap::ValueHint::AnyPath,
+        value_parser = |s: &str| RepoPathBuf::from_relative_path(s),
+    )]
+    remove: Vec<RepoPathBuf>,
     /// Include no files in the working copy (combine with --add)
     #[arg(long)]
     clear: bool,
@@ -89,8 +98,7 @@ fn cmd_sparse_list(
 ) -> Result<(), CommandError> {
     let workspace_command = command.workspace_helper(ui)?;
     for path in workspace_command.working_copy().sparse_patterns()? {
-        let ui_path = workspace_command.format_file_path(path);
+        writeln!(ui.stdout(), "{}", path.to_fs_path(Path::new("")).display())?;
-        writeln!(ui.stdout(), "{ui_path}")?;
     }
     Ok(())
 }
@@ -102,16 +110,6 @@ fn cmd_sparse_set(
     args: &SparseSetArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
-    let paths_to_add: Vec<_> = args
-        .add
-        .iter()
-        .map(|v| workspace_command.parse_file_path(v))
-        .try_collect()?;
-    let paths_to_remove: Vec<_> = args
-        .remove
-        .iter()
-        .map(|v| workspace_command.parse_file_path(v))
-        .try_collect()?;
     let repo_path = workspace_command.repo().repo_path().to_owned();
     let (mut locked_ws, wc_commit) = workspace_command.start_working_copy_mutation()?;
     let mut new_patterns = HashSet::new();
@@ -120,12 +118,12 @@ fn cmd_sparse_set(
     } else {
         if !args.clear {
             new_patterns.extend(locked_ws.locked_wc().sparse_patterns()?.iter().cloned());
-            for path in paths_to_remove {
+            for path in &args.remove {
-                new_patterns.remove(&path);
+                new_patterns.remove(path);
             }
         }
-        for path in paths_to_add {
+        for path in &args.add {
-            new_patterns.insert(path);
+            new_patterns.insert(path.to_owned());
         }
     }
     let mut new_patterns = new_patterns.into_iter().collect_vec();

cli/tests/test_sparse_command.rs

@@ -56,16 +56,29 @@ fn test_sparse_manage_patterns() {
     file3
     "###);
 
+    // Run commands in sub directory to ensure that patterns are parsed as
+    // workspace-relative paths, not cwd-relative ones.
+    let sub_dir = repo_path.join("sub");
+    std::fs::create_dir(&sub_dir).unwrap();
+
+    // Not a workspace-relative path
+    let stderr = test_env.jj_cmd_cli_error(&sub_dir, &["sparse", "set", "--add=../file2"]);
+    insta::assert_snapshot!(stderr, @r###"
+    error: invalid value '../file2' for '--add <ADD>': Invalid component ".." in repo-relative path "../file2"
+
+    For more information, try '--help'.
+    "###);
+
     // Can `--add` a few files
     let (stdout, stderr) = test_env.jj_cmd_ok(
-        &repo_path,
+        &sub_dir,
         &["sparse", "set", "--add", "file2", "--add", "file3"],
     );
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Added 2 files, modified 0 files, removed 0 files
     "###);
-    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "list"]);
+    let stdout = test_env.jj_cmd_success(&sub_dir, &["sparse", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     file2
     file3
@@ -76,7 +89,7 @@ fn test_sparse_manage_patterns() {
 
     // Can combine `--add` and `--remove`
     let (stdout, stderr) = test_env.jj_cmd_ok(
-        &repo_path,
+        &sub_dir,
         &[
             "sparse", "set", "--add", "file1", "--remove", "file2", "--remove", "file3",
         ],
@@ -85,7 +98,7 @@ fn test_sparse_manage_patterns() {
     insta::assert_snapshot!(stderr, @r###"
     Added 1 files, modified 0 files, removed 2 files
     "###);
-    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "list"]);
+    let stdout = test_env.jj_cmd_success(&sub_dir, &["sparse", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     file1
     "###);
@@ -95,12 +108,12 @@ fn test_sparse_manage_patterns() {
 
     // Can use `--clear` and `--add`
     let (stdout, stderr) =
-        test_env.jj_cmd_ok(&repo_path, &["sparse", "set", "--clear", "--add", "file2"]);
+        test_env.jj_cmd_ok(&sub_dir, &["sparse", "set", "--clear", "--add", "file2"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Added 1 files, modified 0 files, removed 1 files
     "###);
-    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "list"]);
+    let stdout = test_env.jj_cmd_success(&sub_dir, &["sparse", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     file2
     "###);
@@ -109,12 +122,12 @@ fn test_sparse_manage_patterns() {
     assert!(!repo_path.join("file3").exists());
 
     // Can reset back to all files
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["sparse", "set", "--reset"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&sub_dir, &["sparse", "set", "--reset"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Added 2 files, modified 0 files, removed 0 files
     "###);
-    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "list"]);
+    let stdout = test_env.jj_cmd_success(&sub_dir, &["sparse", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     .
     "###);
@@ -134,13 +147,13 @@ fn test_sparse_manage_patterns() {
     let read_patterns = || std::fs::read_to_string(test_env.env_root().join("patterns0")).unwrap();
 
     edit_patterns(&["file1"]);
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["sparse", "set", "--edit"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&sub_dir, &["sparse", "set", "--edit"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Added 0 files, modified 0 files, removed 2 files
     "###);
     insta::assert_snapshot!(read_patterns(), @".");
-    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "list"]);
+    let stdout = test_env.jj_cmd_success(&sub_dir, &["sparse", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     file1
     "###);
@@ -148,7 +161,7 @@ fn test_sparse_manage_patterns() {
     // Can edit with `--clear` and `--add`
     edit_patterns(&["file2"]);
     let (stdout, stderr) = test_env.jj_cmd_ok(
-        &repo_path,
+        &sub_dir,
         &["sparse", "set", "--edit", "--clear", "--add", "file1"],
     );
     insta::assert_snapshot!(stdout, @"");
@@ -156,20 +169,20 @@ fn test_sparse_manage_patterns() {
     Added 1 files, modified 0 files, removed 1 files
     "###);
     insta::assert_snapshot!(read_patterns(), @"file1");
-    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "list"]);
+    let stdout = test_env.jj_cmd_success(&sub_dir, &["sparse", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     file2
     "###);
 
     // Can edit with multiple files
     edit_patterns(&["file2", "file3"]);
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["sparse", "set", "--clear", "--edit"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&sub_dir, &["sparse", "set", "--clear", "--edit"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Added 1 files, modified 0 files, removed 0 files
     "###);
     insta::assert_snapshot!(read_patterns(), @"");
-    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "list"]);
+    let stdout = test_env.jj_cmd_success(&sub_dir, &["sparse", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     file2
     file3