cli_util: improve API for editing text in a tempfile

Consolidates bulky tempfile code in sparse.rs and description_util.rs into cli_util.rs

cli/src/cli_util.rs

@@ -17,7 +17,7 @@ use std::collections::{HashMap, HashSet};
 use std::env::{self, ArgsOs, VarError};
 use std::ffi::{OsStr, OsString};
 use std::fmt::Debug;
-use std::io::Write as _;
+use std::io::{self, Write as _};
 use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
@@ -25,7 +25,7 @@ use std::rc::Rc;
 use std::str::FromStr;
 use std::sync::Arc;
 use std::time::SystemTime;
-use std::{iter, str};
+use std::{fs, iter, str};
 
 use clap::builder::{NonEmptyStringValueParser, TypedValueParser, ValueParserFactory};
 use clap::{Arg, ArgAction, ArgMatches, Command, FromArgMatches};
@@ -2278,6 +2278,49 @@ pub fn run_ui_editor(settings: &UserSettings, edit_path: &PathBuf) -> Result<(),
     Ok(())
 }
 
+pub fn edit_temp_file(
+    error_name: &str,
+    tempfile_suffix: &str,
+    dir: &Path,
+    content: &str,
+    settings: &UserSettings,
+) -> Result<String, CommandError> {
+    let path = (|| -> Result<_, io::Error> {
+        let mut file = tempfile::Builder::new()
+            .prefix("editor-")
+            .suffix(tempfile_suffix)
+            .tempfile_in(dir)?;
+        file.write_all(content.as_bytes())?;
+        let (_, path) = file.keep().map_err(|e| e.error)?;
+        Ok(path)
+    })()
+    .map_err(|e| {
+        user_error(format!(
+            "Failed to create {} file in \"{}\": {}",
+            error_name,
+            dir.display(),
+            e
+        ))
+    })?;
+
+    run_ui_editor(settings, &path)?;
+
+    let edited = fs::read_to_string(&path).map_err(|e| {
+        user_error(format!(
+            r#"Failed to read {} file "{}": {}"#,
+            error_name,
+            path.display(),
+            e
+        ))
+    })?;
+
+    // Delete the file only if everything went well.
+    // TODO: Tell the user the name of the file we left behind.
+    std::fs::remove_file(path).ok();
+
+    Ok(edited)
+}
+
 pub fn short_commit_hash(commit_id: &CommitId) -> String {
     commit_id.hex()[0..12].to_string()
 }

cli/src/commands/sparse.rs

@@ -13,7 +13,8 @@
 // limitations under the License.
 
 use std::collections::HashSet;
-use std::io::{self, BufRead, Seek, SeekFrom, Write};
+use std::fmt::Write as _;
+use std::io::{self, Write};
 use std::path::Path;
 
 use clap::Subcommand;
@@ -23,9 +24,7 @@ use jj_lib::repo_path::RepoPathBuf;
 use jj_lib::settings::UserSettings;
 use tracing::instrument;
 
-use crate::cli_util::{
+use crate::cli_util::{edit_temp_file, print_checkout_stats, CommandError, CommandHelper};
-    print_checkout_stats, run_ui_editor, user_error, CommandError, CommandHelper,
-};
 use crate::ui::Ui;
 
 /// Manage which paths from the working-copy commit are present in the working
@@ -164,58 +163,34 @@ fn edit_sparse(
     sparse: &[RepoPathBuf],
     settings: &UserSettings,
 ) -> Result<Vec<RepoPathBuf>, CommandError> {
-    let file = (|| -> Result<_, io::Error> {
+    let mut content = String::new();
-        let mut file = tempfile::Builder::new()
+    for sparse_path in sparse {
-            .prefix("editor-")
+        let workspace_relative_sparse_path =
-            .suffix(".jjsparse")
+            file_util::relative_path(workspace_root, &sparse_path.to_fs_path(workspace_root));
-            .tempfile_in(repo_path)?;
+        let path_string = workspace_relative_sparse_path.to_str().ok_or_else(|| {
-        for sparse_path in sparse {
+            io::Error::new(
-            let workspace_relative_sparse_path =
+                io::ErrorKind::InvalidData,
-                file_util::relative_path(workspace_root, &sparse_path.to_fs_path(workspace_root));
+                format!(
-            file.write_all(
+                    "stored sparse path is not valid utf-8: {}",
-                workspace_relative_sparse_path
+                    workspace_relative_sparse_path.display()
-                    .to_str()
+                ),
-                    .ok_or_else(|| {
+            )
-                        io::Error::new(
+        })?;
-                            io::ErrorKind::InvalidData,
+        writeln!(&mut content, "{}", path_string).unwrap();
-                            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)?;
+    let content = edit_temp_file(
+        "sparse patterns",
+        ".jjsparse",
+        repo_path,
+        &content,
+        settings,
+    )?;
 
-    // Read and parse patterns.
+    content
-    io::BufReader::new(file)
         .lines()
-        .filter(|line| {
+        .filter(|line| !line.starts_with("JJ: ") && !line.trim().is_empty())
-            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>(RepoPathBuf::parse_fs_path(
                 workspace_root,
                 workspace_root,

cli/src/description_util.rs

@@ -1,6 +1,3 @@
-use std::io::Write;
-use std::{fs, io};
-
 use itertools::Itertools;
 use jj_lib::commit::Commit;
 use jj_lib::matchers::EverythingMatcher;
@@ -8,7 +5,7 @@ use jj_lib::merged_tree::MergedTree;
 use jj_lib::repo::ReadonlyRepo;
 use jj_lib::settings::UserSettings;
 
-use crate::cli_util::{run_ui_editor, user_error, CommandError, WorkspaceCommandHelper};
+use crate::cli_util::{edit_temp_file, CommandError, WorkspaceCommandHelper};
 use crate::diff_util::{self, DiffFormat};
 use crate::formatter::PlainTextFormatter;
 use crate::text_util;
@@ -19,34 +16,21 @@ pub fn edit_description(
     description: &str,
     settings: &UserSettings,
 ) -> Result<String, CommandError> {
-    let description_file_path = (|| -> Result<_, io::Error> {
+    let description = format!(
-        let mut file = tempfile::Builder::new()
+        r#"{}
-            .prefix("editor-")
+JJ: Lines starting with "JJ: " (like this one) will be removed.
-            .suffix(".jjdescription")
+"#,
-            .tempfile_in(repo.repo_path())?;
+        description
-        file.write_all(description.as_bytes())?;
+    );
-        file.write_all(b"\nJJ: Lines starting with \"JJ: \" (like this one) will be removed.\n")?;
-        let (_, path) = file.keep().map_err(|e| e.error)?;
-        Ok(path)
-    })()
-    .map_err(|e| {
-        user_error(format!(
-            r#"Failed to create description file in "{path}": {e}"#,
-            path = repo.repo_path().display()
-        ))
-    })?;
 
-    run_ui_editor(settings, &description_file_path)?;
+    let description = edit_temp_file(
+        "description",
+        ".jjdescription",
+        repo.repo_path(),
+        &description,
+        settings,
+    )?;
 
-    let description = fs::read_to_string(&description_file_path).map_err(|e| {
-        user_error(format!(
-            r#"Failed to read description file "{path}": {e}"#,
-            path = description_file_path.display()
-        ))
-    })?;
-    // Delete the file only if everything went well.
-    // TODO: Tell the user the name of the file we left behind.
-    std::fs::remove_file(description_file_path).ok();
     // Normalize line ending, remove leading and trailing blank lines.
     let description = description
         .lines()