From 91a679d5fb5b2d7e00a32557efb0f2f2399cbbe1 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 28 Mar 2024 20:28:20 +0900
Subject: [PATCH 1/5] 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                     |  3 +++
 cli/src/commands/sparse.rs       | 38 ++++++++++++++---------------
 cli/tests/test_sparse_command.rs | 41 +++++++++++++++++++++-----------
 3 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 173297b186..47ebc88fad 100644
--- a/CHANGELOG.md
+++ b/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.
diff --git a/cli/src/commands/sparse.rs b/cli/src/commands/sparse.rs
index 6c0fb351ca..6bb426ecbc 100644
--- a/cli/src/commands/sparse.rs
+++ b/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)]
-    add: Vec<String>,
+    #[arg(
+        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)]
-    remove: Vec<String>,
+    #[arg(
+        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(), "{ui_path}")?;
+        writeln!(ui.stdout(), "{}", path.to_fs_path(Path::new("")).display())?;
     }
     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 {
-                new_patterns.remove(&path);
+            for path in &args.remove {
+                new_patterns.remove(path);
             }
         }
-        for path in paths_to_add {
-            new_patterns.insert(path);
+        for path in &args.add {
+            new_patterns.insert(path.to_owned());
         }
     }
     let mut new_patterns = new_patterns.into_iter().collect_vec();
diff --git a/cli/tests/test_sparse_command.rs b/cli/tests/test_sparse_command.rs
index b40fd66ef5..f641a4e18c 100644
--- a/cli/tests/test_sparse_command.rs
+++ b/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

From ae176557e5abe00ab4409210c08b9e31a4734d5f Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 28 Mar 2024 20:52:25 +0900
Subject: [PATCH 2/5] sparse: extract helper that updates sparse patterns
 within workspace lock

I assumed locked_wc.sparse_patterns() is cheap operation. If it isn't, a
locked_ws/wc should be passed to the callback instead.
---
 cli/src/commands/sparse.rs | 68 ++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/cli/src/commands/sparse.rs b/cli/src/commands/sparse.rs
index 6bb426ecbc..edd0145676 100644
--- a/cli/src/commands/sparse.rs
+++ b/cli/src/commands/sparse.rs
@@ -23,7 +23,9 @@ use jj_lib::repo_path::RepoPathBuf;
 use jj_lib::settings::UserSettings;
 use tracing::instrument;
 
-use crate::cli_util::{edit_temp_file, print_checkout_stats, CommandHelper};
+use crate::cli_util::{
+    edit_temp_file, print_checkout_stats, CommandHelper, WorkspaceCommandHelper,
+};
 use crate::command_error::{
     internal_error, internal_error_with_message, user_error_with_message, CommandError,
 };
@@ -111,36 +113,29 @@ fn cmd_sparse_set(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     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();
-    if args.reset {
-        new_patterns.insert(RepoPathBuf::root());
-    } else {
-        if !args.clear {
-            new_patterns.extend(locked_ws.locked_wc().sparse_patterns()?.iter().cloned());
-            for path in &args.remove {
-                new_patterns.remove(path);
+    update_sparse_patterns_with(ui, &mut workspace_command, |_ui, old_patterns| {
+        let mut new_patterns = HashSet::new();
+        if args.reset {
+            new_patterns.insert(RepoPathBuf::root());
+        } else {
+            if !args.clear {
+                new_patterns.extend(old_patterns.iter().cloned());
+                for path in &args.remove {
+                    new_patterns.remove(path);
+                }
+            }
+            for path in &args.add {
+                new_patterns.insert(path.to_owned());
             }
         }
-        for path in &args.add {
-            new_patterns.insert(path.to_owned());
-        }
-    }
-    let mut new_patterns = new_patterns.into_iter().collect_vec();
-    new_patterns.sort();
-    if args.edit {
-        new_patterns = edit_sparse(&repo_path, &new_patterns, command.settings())?;
+        let mut new_patterns = new_patterns.into_iter().collect_vec();
         new_patterns.sort();
-    }
-    let stats = locked_ws
-        .locked_wc()
-        .set_sparse_patterns(new_patterns)
-        .map_err(|err| internal_error_with_message("Failed to update working copy paths", err))?;
-    let operation_id = locked_ws.locked_wc().old_operation_id().clone();
-    locked_ws.finish(operation_id)?;
-    print_checkout_stats(ui, stats, &wc_commit)?;
-
-    Ok(())
+        if args.edit {
+            new_patterns = edit_sparse(&repo_path, &new_patterns, command.settings())?;
+            new_patterns.sort();
+        }
+        Ok(new_patterns)
+    })
 }
 
 fn edit_sparse(
@@ -180,3 +175,20 @@ fn edit_sparse(
         })
         .try_collect()
 }
+
+fn update_sparse_patterns_with(
+    ui: &mut Ui,
+    workspace_command: &mut WorkspaceCommandHelper,
+    f: impl FnOnce(&mut Ui, &[RepoPathBuf]) -> Result<Vec<RepoPathBuf>, CommandError>,
+) -> Result<(), CommandError> {
+    let (mut locked_ws, wc_commit) = workspace_command.start_working_copy_mutation()?;
+    let new_patterns = f(ui, locked_ws.locked_wc().sparse_patterns()?)?;
+    let stats = locked_ws
+        .locked_wc()
+        .set_sparse_patterns(new_patterns)
+        .map_err(|err| internal_error_with_message("Failed to update working copy paths", err))?;
+    let operation_id = locked_ws.locked_wc().old_operation_id().clone();
+    locked_ws.finish(operation_id)?;
+    print_checkout_stats(ui, stats, &wc_commit)?;
+    Ok(())
+}

From a49f64e02870a53a6d2547169bb99cc140468951 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 28 Mar 2024 21:10:47 +0900
Subject: [PATCH 3/5] sparse: extract "set --edit" to subcommand

Even though --edit can be combined with --add/--remove/--clear/--reset, I don't
think it's practically useful.
---
 CHANGELOG.md                     |  2 ++
 cli/src/commands/sparse.rs       | 31 +++++++++++++++++++++----------
 cli/tests/cli-reference@.md.snap | 12 +++++++++---
 cli/tests/test_sparse_command.rs | 28 +++++++---------------------
 4 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 47ebc88fad..39d2693c61 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,8 @@ 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 set --edit` was split up into `jj sparse edit`.
+
 * The `jj sparse` subcommands now parse and print patterns as workspace-relative
   paths.
 
diff --git a/cli/src/commands/sparse.rs b/cli/src/commands/sparse.rs
index edd0145676..4a2e107841 100644
--- a/cli/src/commands/sparse.rs
+++ b/cli/src/commands/sparse.rs
@@ -37,6 +37,7 @@ use crate::ui::Ui;
 pub(crate) enum SparseArgs {
     List(SparseListArgs),
     Set(SparseSetArgs),
+    Edit(SparseEditArgs),
 }
 
 /// List the patterns that are currently present in the working copy
@@ -72,14 +73,15 @@ pub(crate) struct SparseSetArgs {
     /// Include no files in the working copy (combine with --add)
     #[arg(long)]
     clear: bool,
-    /// Edit patterns with $EDITOR
-    #[arg(long)]
-    edit: bool,
     /// Include all files in the working copy
     #[arg(long, conflicts_with_all = &["add", "remove", "clear"])]
     reset: bool,
 }
 
+/// Start an editor to update the patterns that are present in the working copy
+#[derive(clap::Args, Clone, Debug)]
+pub(crate) struct SparseEditArgs {}
+
 #[instrument(skip_all)]
 pub(crate) fn cmd_sparse(
     ui: &mut Ui,
@@ -89,6 +91,7 @@ pub(crate) fn cmd_sparse(
     match args {
         SparseArgs::List(sub_args) => cmd_sparse_list(ui, command, sub_args),
         SparseArgs::Set(sub_args) => cmd_sparse_set(ui, command, sub_args),
+        SparseArgs::Edit(sub_args) => cmd_sparse_edit(ui, command, sub_args),
     }
 }
 
@@ -112,7 +115,6 @@ fn cmd_sparse_set(
     args: &SparseSetArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
-    let repo_path = workspace_command.repo().repo_path().to_owned();
     update_sparse_patterns_with(ui, &mut workspace_command, |_ui, old_patterns| {
         let mut new_patterns = HashSet::new();
         if args.reset {
@@ -128,12 +130,21 @@ fn cmd_sparse_set(
                 new_patterns.insert(path.to_owned());
             }
         }
-        let mut new_patterns = new_patterns.into_iter().collect_vec();
-        new_patterns.sort();
-        if args.edit {
-            new_patterns = edit_sparse(&repo_path, &new_patterns, command.settings())?;
-            new_patterns.sort();
-        }
+        Ok(new_patterns.into_iter().sorted_unstable().collect())
+    })
+}
+
+#[instrument(skip_all)]
+fn cmd_sparse_edit(
+    ui: &mut Ui,
+    command: &CommandHelper,
+    _args: &SparseEditArgs,
+) -> Result<(), CommandError> {
+    let mut workspace_command = command.workspace_helper(ui)?;
+    let repo_path = workspace_command.repo().repo_path().to_owned();
+    update_sparse_patterns_with(ui, &mut workspace_command, |_ui, old_patterns| {
+        let mut new_patterns = edit_sparse(&repo_path, old_patterns, command.settings())?;
+        new_patterns.sort_unstable();
         Ok(new_patterns)
     })
 }
diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap
index 420f3a616d..a645a49833 100644
--- a/cli/tests/cli-reference@.md.snap
+++ b/cli/tests/cli-reference@.md.snap
@@ -70,6 +70,7 @@ This document contains the help content for the `jj` command-line program.
 * [`jj sparse`↴](#jj-sparse)
 * [`jj sparse list`↴](#jj-sparse-list)
 * [`jj sparse set`↴](#jj-sparse-set)
+* [`jj sparse edit`↴](#jj-sparse-edit)
 * [`jj split`↴](#jj-split)
 * [`jj squash`↴](#jj-squash)
 * [`jj status`↴](#jj-status)
@@ -1623,6 +1624,7 @@ Manage which paths from the working-copy commit are present in the working copy
 
 * `list` — List the patterns that are currently present in the working copy
 * `set` — Update the patterns that are present in the working copy
+* `edit` — Start an editor to update the patterns that are present in the working copy
 
 
 
@@ -1652,15 +1654,19 @@ For example, if all you need is the `README.md` and the `lib/` directory, use `j
 
   Possible values: `true`, `false`
 
-* `--edit` — Edit patterns with $EDITOR
+* `--reset` — Include all files in the working copy
 
   Possible values: `true`, `false`
 
-* `--reset` — Include all files in the working copy
 
-  Possible values: `true`, `false`
 
 
+## `jj sparse edit`
+
+Start an editor to update the patterns that are present in the working copy
+
+**Usage:** `jj sparse edit`
+
 
 
 ## `jj split`
diff --git a/cli/tests/test_sparse_command.rs b/cli/tests/test_sparse_command.rs
index f641a4e18c..c62f65007b 100644
--- a/cli/tests/test_sparse_command.rs
+++ b/cli/tests/test_sparse_command.rs
@@ -147,7 +147,7 @@ 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(&sub_dir, &["sparse", "set", "--edit"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&sub_dir, &["sparse", "edit"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Added 0 files, modified 0 files, removed 2 files
@@ -158,30 +158,16 @@ fn test_sparse_manage_patterns() {
     file1
     "###);
 
-    // Can edit with `--clear` and `--add`
-    edit_patterns(&["file2"]);
-    let (stdout, stderr) = test_env.jj_cmd_ok(
-        &sub_dir,
-        &["sparse", "set", "--edit", "--clear", "--add", "file1"],
-    );
-    insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
-    Added 1 files, modified 0 files, removed 1 files
-    "###);
-    insta::assert_snapshot!(read_patterns(), @"file1");
-    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(&sub_dir, &["sparse", "set", "--clear", "--edit"]);
+    edit_patterns(&["file3", "file2"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&sub_dir, &["sparse", "edit"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
-    Added 1 files, modified 0 files, removed 0 files
+    Added 2 files, modified 0 files, removed 1 files
+    "###);
+    insta::assert_snapshot!(read_patterns(), @r###"
+    file1
     "###);
-    insta::assert_snapshot!(read_patterns(), @"");
     let stdout = test_env.jj_cmd_success(&sub_dir, &["sparse", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     file2

From 15f9865c5c8d6a790dafbaeae50af28fc910cb98 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 28 Mar 2024 23:14:01 +0900
Subject: [PATCH 4/5] sparse: deduplicate edited patterns

Since "set --add" removes duplicated patterns, it makes sense for "edit" to do
the same thing.
---
 cli/src/commands/sparse.rs       | 1 +
 cli/tests/test_sparse_command.rs | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/cli/src/commands/sparse.rs b/cli/src/commands/sparse.rs
index 4a2e107841..01a4433da2 100644
--- a/cli/src/commands/sparse.rs
+++ b/cli/src/commands/sparse.rs
@@ -145,6 +145,7 @@ fn cmd_sparse_edit(
     update_sparse_patterns_with(ui, &mut workspace_command, |_ui, old_patterns| {
         let mut new_patterns = edit_sparse(&repo_path, old_patterns, command.settings())?;
         new_patterns.sort_unstable();
+        new_patterns.dedup();
         Ok(new_patterns)
     })
 }
diff --git a/cli/tests/test_sparse_command.rs b/cli/tests/test_sparse_command.rs
index c62f65007b..6ad47880bc 100644
--- a/cli/tests/test_sparse_command.rs
+++ b/cli/tests/test_sparse_command.rs
@@ -159,7 +159,7 @@ fn test_sparse_manage_patterns() {
     "###);
 
     // Can edit with multiple files
-    edit_patterns(&["file3", "file2"]);
+    edit_patterns(&["file3", "file2", "file3"]);
     let (stdout, stderr) = test_env.jj_cmd_ok(&sub_dir, &["sparse", "edit"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"

From b4d576401cca87a2d7b8585bbf61d7ea298eef68 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 28 Mar 2024 21:19:24 +0900
Subject: [PATCH 5/5] sparse: extract "set --reset" to subcommand

Since --reset conflicts with the other flags, "set --reset" seems odd.
---
 CHANGELOG.md                     |  3 ++-
 cli/src/commands/sparse.rs       | 39 ++++++++++++++++++++------------
 cli/tests/cli-reference@.md.snap | 10 ++++++--
 cli/tests/test_sparse_command.rs |  2 +-
 4 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 39d2693c61..f6b10200ca 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,7 +16,8 @@ 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 set --edit` was split up into `jj sparse edit`.
+* The `jj sparse set --edit`/`--reset` flags were split up into `jj sparse
+  edit`/`reset` subcommands respectively.
 
 * The `jj sparse` subcommands now parse and print patterns as workspace-relative
   paths.
diff --git a/cli/src/commands/sparse.rs b/cli/src/commands/sparse.rs
index 01a4433da2..9b4b79b186 100644
--- a/cli/src/commands/sparse.rs
+++ b/cli/src/commands/sparse.rs
@@ -37,6 +37,7 @@ use crate::ui::Ui;
 pub(crate) enum SparseArgs {
     List(SparseListArgs),
     Set(SparseSetArgs),
+    Reset(SparseResetArgs),
     Edit(SparseEditArgs),
 }
 
@@ -73,11 +74,12 @@ pub(crate) struct SparseSetArgs {
     /// Include no files in the working copy (combine with --add)
     #[arg(long)]
     clear: bool,
-    /// Include all files in the working copy
-    #[arg(long, conflicts_with_all = &["add", "remove", "clear"])]
-    reset: bool,
 }
 
+/// Reset the patterns to include all files in the working copy
+#[derive(clap::Args, Clone, Debug)]
+pub(crate) struct SparseResetArgs {}
+
 /// Start an editor to update the patterns that are present in the working copy
 #[derive(clap::Args, Clone, Debug)]
 pub(crate) struct SparseEditArgs {}
@@ -91,6 +93,7 @@ pub(crate) fn cmd_sparse(
     match args {
         SparseArgs::List(sub_args) => cmd_sparse_list(ui, command, sub_args),
         SparseArgs::Set(sub_args) => cmd_sparse_set(ui, command, sub_args),
+        SparseArgs::Reset(sub_args) => cmd_sparse_reset(ui, command, sub_args),
         SparseArgs::Edit(sub_args) => cmd_sparse_edit(ui, command, sub_args),
     }
 }
@@ -117,23 +120,31 @@ fn cmd_sparse_set(
     let mut workspace_command = command.workspace_helper(ui)?;
     update_sparse_patterns_with(ui, &mut workspace_command, |_ui, old_patterns| {
         let mut new_patterns = HashSet::new();
-        if args.reset {
-            new_patterns.insert(RepoPathBuf::root());
-        } else {
-            if !args.clear {
-                new_patterns.extend(old_patterns.iter().cloned());
-                for path in &args.remove {
-                    new_patterns.remove(path);
-                }
-            }
-            for path in &args.add {
-                new_patterns.insert(path.to_owned());
+        if !args.clear {
+            new_patterns.extend(old_patterns.iter().cloned());
+            for path in &args.remove {
+                new_patterns.remove(path);
             }
         }
+        for path in &args.add {
+            new_patterns.insert(path.to_owned());
+        }
         Ok(new_patterns.into_iter().sorted_unstable().collect())
     })
 }
 
+#[instrument(skip_all)]
+fn cmd_sparse_reset(
+    ui: &mut Ui,
+    command: &CommandHelper,
+    _args: &SparseResetArgs,
+) -> Result<(), CommandError> {
+    let mut workspace_command = command.workspace_helper(ui)?;
+    update_sparse_patterns_with(ui, &mut workspace_command, |_ui, _old_patterns| {
+        Ok(vec![RepoPathBuf::root()])
+    })
+}
+
 #[instrument(skip_all)]
 fn cmd_sparse_edit(
     ui: &mut Ui,
diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap
index a645a49833..566fb780e7 100644
--- a/cli/tests/cli-reference@.md.snap
+++ b/cli/tests/cli-reference@.md.snap
@@ -70,6 +70,7 @@ This document contains the help content for the `jj` command-line program.
 * [`jj sparse`↴](#jj-sparse)
 * [`jj sparse list`↴](#jj-sparse-list)
 * [`jj sparse set`↴](#jj-sparse-set)
+* [`jj sparse reset`↴](#jj-sparse-reset)
 * [`jj sparse edit`↴](#jj-sparse-edit)
 * [`jj split`↴](#jj-split)
 * [`jj squash`↴](#jj-squash)
@@ -1624,6 +1625,7 @@ Manage which paths from the working-copy commit are present in the working copy
 
 * `list` — List the patterns that are currently present in the working copy
 * `set` — Update the patterns that are present in the working copy
+* `reset` — Reset the patterns to include all files in the working copy
 * `edit` — Start an editor to update the patterns that are present in the working copy
 
 
@@ -1654,11 +1656,15 @@ For example, if all you need is the `README.md` and the `lib/` directory, use `j
 
   Possible values: `true`, `false`
 
-* `--reset` — Include all files in the working copy
 
-  Possible values: `true`, `false`
 
 
+## `jj sparse reset`
+
+Reset the patterns to include all files in the working copy
+
+**Usage:** `jj sparse reset`
+
 
 
 ## `jj sparse edit`
diff --git a/cli/tests/test_sparse_command.rs b/cli/tests/test_sparse_command.rs
index 6ad47880bc..7c2a816f7b 100644
--- a/cli/tests/test_sparse_command.rs
+++ b/cli/tests/test_sparse_command.rs
@@ -122,7 +122,7 @@ 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(&sub_dir, &["sparse", "set", "--reset"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&sub_dir, &["sparse", "reset"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Added 2 files, modified 0 files, removed 0 files