add --edit option to jj sparse set

CHANGELOG.md

@@ -116,6 +116,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   on arbitrary revisions. Bits other than the executable bit are not planned to
   be supported.
 
+* `jj sparse set` now accepts an `--edit` flag which brings up the `$EDITOR` to
+  edit sparse patterns.
+
 ### Fixed bugs
 
 * Modify/delete conflicts now include context lines

src/commands/mod.rs

@@ -21,8 +21,8 @@ mod operation;
 
 use std::collections::{BTreeMap, HashSet};
 use std::fmt::Debug;
-use std::io::{Read, Seek, SeekFrom, Write};
-use std::path::PathBuf;
+use std::io::{BufRead, Read, Seek, SeekFrom, Write};
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::{fs, io};
 
@@ -953,6 +953,9 @@ 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,
@@ -1876,6 +1879,73 @@ fn edit_description(
     Ok(text_util::complete_newline(description))
 }
 
+fn edit_sparse(
+    workspace_root: &Path,
+    repo_path: &Path,
+    sparse: &[RepoPath],
+    settings: &UserSettings,
+) -> Result<Vec<RepoPath>, CommandError> {
+    let file = (|| -> Result<_, io::Error> {
+        let mut file = tempfile::Builder::new()
+            .prefix("editor-")
+            .suffix(".jjsparse")
+            .tempfile_in(repo_path)?;
+        for sparse_path in sparse {
+            let workspace_relative_sparse_path =
+                file_util::relative_path(workspace_root, &sparse_path.to_fs_path(workspace_root));
+            file.write_all(
+                workspace_relative_sparse_path
+                    .to_str()
+                    .ok_or_else(|| {
+                        io::Error::new(
+                            io::ErrorKind::InvalidData,
+                            format!(
+                                "stored sparse path is not valid utf-8: {}",
+                                workspace_relative_sparse_path.display()
+                            ),
+                        )
+                    })?
+                    .as_bytes(),
+            )?;
+            file.write_all(b"\n")?;
+        }
+        file.seek(SeekFrom::Start(0))?;
+        Ok(file)
+    })()
+    .map_err(|e| {
+        user_error(format!(
+            r#"Failed to create sparse patterns file in "{path}": {e}"#,
+            path = repo_path.display()
+        ))
+    })?;
+    let file_path = file.path().to_owned();
+
+    run_ui_editor(settings, &file_path)?;
+
+    // Read and parse patterns.
+    io::BufReader::new(file)
+        .lines()
+        .filter(|line| {
+            line.as_ref()
+                .map(|line| !line.starts_with("JJ: ") && !line.trim().is_empty())
+                .unwrap_or(true)
+        })
+        .map(|line| {
+            let line = line.map_err(|e| {
+                user_error(format!(
+                    r#"Failed to read sparse patterns file "{path}": {e}"#,
+                    path = file_path.display()
+                ))
+            })?;
+            Ok::<_, CommandError>(RepoPath::parse_fs_path(
+                workspace_root,
+                workspace_root,
+                line.trim(),
+            )?)
+        })
+        .try_collect()
+}
+
 fn cmd_describe(
     ui: &mut Ui,
     command: &CommandHelper,
@@ -3507,6 +3577,14 @@ fn cmd_sparse_set(
         .iter()
         .map(|v| workspace_command.parse_file_path(v))
         .try_collect()?;
+    // Determine inputs of `edit` operation now, since `workspace_command` is
+    // inaccessible while the working copy is locked.
+    let edit_inputs = args.edit.then(|| {
+        (
+            workspace_command.repo().clone(),
+            workspace_command.workspace_root().clone(),
+        )
+    });
     let (mut locked_wc, _wc_commit) = workspace_command.start_working_copy_mutation()?;
     let mut new_patterns = HashSet::new();
     if args.reset {
@@ -3522,7 +3600,17 @@ fn cmd_sparse_set(
             new_patterns.insert(path);
         }
     }
-    let new_patterns = new_patterns.into_iter().sorted().collect();
+    let mut new_patterns = new_patterns.into_iter().collect_vec();
+    new_patterns.sort();
+    if let Some((repo, workspace_root)) = edit_inputs {
+        new_patterns = edit_sparse(
+            &workspace_root,
+            repo.repo_path(),
+            &new_patterns,
+            command.settings(),
+        )?;
+        new_patterns.sort();
+    }
     let stats = locked_wc.set_sparse_patterns(new_patterns).map_err(|err| {
         CommandError::InternalError(format!("Failed to update working copy paths: {err}"))
     })?;

tests/test_sparse_command.rs

@@ -12,16 +12,20 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::io::Write;
+
 use crate::common::TestEnvironment;
 
 pub mod common;
 
 #[test]
 fn test_sparse_manage_patterns() {
-    let test_env = TestEnvironment::default();
+    let mut test_env = TestEnvironment::default();
     test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
     let repo_path = test_env.env_root().join("repo");
 
+    let edit_script = test_env.set_up_fake_editor();
+
     // Write some files to the working copy
     std::fs::write(repo_path.join("file1"), "contents").unwrap();
     std::fs::write(repo_path.join("file2"), "contents").unwrap();
@@ -114,4 +118,54 @@ fn test_sparse_manage_patterns() {
     assert!(repo_path.join("file1").exists());
     assert!(repo_path.join("file2").exists());
     assert!(repo_path.join("file3").exists());
+
+    // Can edit with editor
+    let edit_patterns = |patterns: &[&str]| {
+        let mut file = std::fs::File::create(&edit_script).unwrap();
+        file.write_all(b"dump patterns0\0write\n").unwrap();
+        for pattern in patterns {
+            file.write_all(pattern.as_bytes()).unwrap();
+            file.write_all(b"\n").unwrap();
+        }
+    };
+    let read_patterns = || std::fs::read_to_string(test_env.env_root().join("patterns0")).unwrap();
+
+    edit_patterns(&["file1"]);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "set", "--edit"]);
+    insta::assert_snapshot!(stdout, @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"]);
+    insta::assert_snapshot!(stdout, @r###"
+    file1
+    "###);
+
+    // Can edit with `--clear` and `--add`
+    edit_patterns(&["file2"]);
+    let stdout = test_env.jj_cmd_success(
+        &repo_path,
+        &["sparse", "set", "--edit", "--clear", "--add", "file1"],
+    );
+    insta::assert_snapshot!(stdout, @r###"
+    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"]);
+    insta::assert_snapshot!(stdout, @r###"
+    file2
+    "###);
+
+    // Can edit with multiple files
+    edit_patterns(&["file2", "file3"]);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["sparse", "set", "--clear", "--edit"]);
+    insta::assert_snapshot!(stdout, @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"]);
+    insta::assert_snapshot!(stdout, @r###"
+    file2
+    file3
+    "###);
 }