From 4024fb4880c62820f5d412d9c6181f4af92ffec1 Mon Sep 17 00:00:00 2001 From: Daniel Ploch Date: Mon, 22 Jan 2024 17:06:47 -0500 Subject: [PATCH] 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 | 47 +++++++++++++++++++++- cli/src/commands/sparse.rs | 79 +++++++++++++------------------------ cli/src/description_util.rs | 44 +++++++-------------- 3 files changed, 86 insertions(+), 84 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 3d5e59b3d..078f69c62 100644 --- a/cli/src/cli_util.rs +++ b/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 { + 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() } diff --git a/cli/src/commands/sparse.rs b/cli/src/commands/sparse.rs index df81995c5..be90c72ad 100644 --- a/cli/src/commands/sparse.rs +++ b/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::{ - print_checkout_stats, run_ui_editor, user_error, CommandError, CommandHelper, -}; +use crate::cli_util::{edit_temp_file, print_checkout_stats, 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, 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(); + let mut content = String::new(); + for sparse_path in sparse { + let workspace_relative_sparse_path = + file_util::relative_path(workspace_root, &sparse_path.to_fs_path(workspace_root)); + let path_string = 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() + ), + ) + })?; + writeln!(&mut content, "{}", path_string).unwrap(); + } - run_ui_editor(settings, &file_path)?; + let content = edit_temp_file( + "sparse patterns", + ".jjsparse", + repo_path, + &content, + settings, + )?; - // Read and parse patterns. - io::BufReader::new(file) + content .lines() - .filter(|line| { - line.as_ref() - .map(|line| !line.starts_with("JJ: ") && !line.trim().is_empty()) - .unwrap_or(true) - }) + .filter(|line| !line.starts_with("JJ: ") && !line.trim().is_empty()) .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, diff --git a/cli/src/description_util.rs b/cli/src/description_util.rs index 606ef611a..745ab53ba 100644 --- a/cli/src/description_util.rs +++ b/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 { - let description_file_path = (|| -> Result<_, io::Error> { - let mut file = tempfile::Builder::new() - .prefix("editor-") - .suffix(".jjdescription") - .tempfile_in(repo.repo_path())?; - 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() - )) - })?; + let description = format!( + r#"{} +JJ: Lines starting with "JJ: " (like this one) will be removed. +"#, + description + ); - 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()