diff --git a/crates/radicle-cli/src/commands/config.rs b/crates/radicle-cli/src/commands/config.rs index cfd32531..402e5a46 100644 --- a/crates/radicle-cli/src/commands/config.rs +++ b/crates/radicle-cli/src/commands/config.rs @@ -185,12 +185,23 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { path.display() ); } - Operation::Edit => match term::editor::Editor::new(&path)?.extension("json").edit()? { - Some(_) => { - term::success!("Successfully made changes to the configuration at {path:?}") + Operation::Edit => { + let config = std::fs::read_to_string(&path)?; + + match term::editor::Editor::new("Change configuration.", "You may change the Radicle configuration using your editor. Pressing (e) will open the editor. Save the file and exit the editor to submit your changes.") + .editor(term::editor::default_editor_command().as_ref()) + .extension(".json") + .initial(&config) + .edit()? { + Some(edited) => { + std::fs::write(&path, edited)?; + term::success!("Successfully made changes to the configuration at {path:?}"); + }, + None => { + term::info!("No changes were made to the configuration at {path:?}") + } } - None => term::info!("No changes were made to the configuration at {path:?}"), - }, + } } Ok(()) diff --git a/crates/radicle-cli/src/commands/id.rs b/crates/radicle-cli/src/commands/id.rs index 2690454e..a01ce322 100644 --- a/crates/radicle-cli/src/commands/id.rs +++ b/crates/radicle-cli/src/commands/id.rs @@ -412,8 +412,9 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { // If `--edit` is specified, the document can also be edited via a text edit. let proposal = if edit { match term::editor::Editor::comment() - .extension("json") - .initial(serde_json::to_string_pretty(¤t.doc)?)? + .editor(term::editor::default_editor_command().as_ref()) + .extension(".json") + .initial(&serde_json::to_string_pretty(¤t.doc)?) .edit()? { Some(proposal) => serde_json::from_str::(&proposal)?, diff --git a/crates/radicle-cli/src/commands/patch/review/builder.rs b/crates/radicle-cli/src/commands/patch/review/builder.rs index fe7f2448..dbbd7df9 100644 --- a/crates/radicle-cli/src/commands/patch/review/builder.rs +++ b/crates/radicle-cli/src/commands/patch/review/builder.rs @@ -836,6 +836,8 @@ enum Error { Format(#[from] std::fmt::Error), #[error(transparent)] Git(#[from] git::raw::Error), + #[error("editor error: {0}")] + Editor(#[from] term::editor::Error), } #[derive(Debug)] @@ -860,8 +862,9 @@ impl CommentBuilder { writeln!(&mut input, "> {line}")?; } let output = term::Editor::comment() - .extension("diff") - .initial(input)? + .editor(term::editor::default_editor_command().as_ref()) + .extension(".diff") + .initial(&input) .edit()?; if let Some(output) = output { diff --git a/crates/radicle-cli/src/terminal/issue.rs b/crates/radicle-cli/src/terminal/issue.rs index 518608ad..d2857e84 100644 --- a/crates/radicle-cli/src/terminal/issue.rs +++ b/crates/radicle-cli/src/terminal/issue.rs @@ -1,5 +1,3 @@ -use std::io; - use radicle_term::table::TableOptions; use radicle_term::{Table, VStack}; @@ -34,7 +32,7 @@ pub enum Format { pub fn get_title_description( title: Option, description: Option, -) -> io::Result> { +) -> Result, term::patch::Error> { term::patch::Message::edit_title_description(title, description, OPEN_MSG) } diff --git a/crates/radicle-cli/src/terminal/patch.rs b/crates/radicle-cli/src/terminal/patch.rs index 3a40bb37..7a2e1f9c 100644 --- a/crates/radicle-cli/src/terminal/patch.rs +++ b/crates/radicle-cli/src/terminal/patch.rs @@ -31,6 +31,10 @@ pub enum Error { Io(#[from] io::Error), #[error("invalid utf-8 string")] InvalidUtf8, + #[error("editor error: {0}")] + Editor(#[from] term::editor::Error), + #[error("a patch title must be provided")] + PatchTitleMissing, } /// The user supplied `Patch` description. @@ -47,13 +51,14 @@ pub enum Message { impl Message { /// Get the `Message` as a string according to the method. - pub fn get(self, help: &str) -> std::io::Result { + pub fn get(self, help: &str) -> Result { let comment = match self { Message::Edit => { if io::stderr().is_terminal() { term::Editor::comment() - .extension("markdown") - .initial(help)? + .editor(term::editor::default_editor_command().as_ref()) + .extension(".md") + .initial(help) .edit()? } else { Some(help.to_owned()) @@ -75,7 +80,7 @@ impl Message { title: Option, description: Option, help: &str, - ) -> std::io::Result> { + ) -> Result, Error> { let mut placeholder = String::new(); if let Some(title) = title { @@ -228,11 +233,7 @@ pub fn get_create_message( let (title, description) = (title.trim().to_string(), description.trim().to_string()); if title.is_empty() { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "a patch title must be provided", - ) - .into()); + return Err(Error::PatchTitleMissing); } Ok((title, description)) @@ -249,7 +250,7 @@ fn edit_display_message(title: &str, description: &str) -> String { pub fn get_edit_message( patch_message: term::patch::Message, patch: &cob::patch::Patch, -) -> io::Result<(String, String)> { +) -> Result<(String, String), Error> { let display_msg = edit_display_message(patch.title(), patch.description()); let patch_message = patch_message.get(&display_msg)?; let patch_message = patch_message.replace(PATCH_MSG.trim(), ""); // Delete help message. @@ -260,10 +261,7 @@ pub fn get_edit_message( let (title, description) = (title.trim().to_string(), description.trim().to_string()); if title.is_empty() { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "a patch title must be provided", - )); + return Err(Error::PatchTitleMissing); } Ok((title, description)) diff --git a/crates/radicle-term/src/editor.rs b/crates/radicle-term/src/editor.rs index d62610a1..9a1ba68c 100644 --- a/crates/radicle-term/src/editor.rs +++ b/crates/radicle-term/src/editor.rs @@ -1,184 +1,67 @@ +use std::env; use std::ffi::OsString; -use std::io::IsTerminal; -use std::io::Write; -use std::os::fd::{AsRawFd, FromRawFd}; -use std::path::{Path, PathBuf}; -use std::process; -use std::{env, fs, io}; +use std::path::Path; -pub const COMMENT_FILE: &str = "RAD_COMMENT"; -/// Some common paths where system-installed binaries are found. -pub const PATHS: &[&str] = &["/usr/local/bin", "/usr/bin", "/bin"]; +use inquire::InquireError; +use thiserror::Error; /// Allows for text input in the configured editor. -pub struct Editor { - path: PathBuf, - truncate: bool, - cleanup: bool, -} +pub struct Editor<'a>(pub inquire::Editor<'a>); -impl Default for Editor { - fn default() -> Self { - Self::comment() - } -} - -impl Drop for Editor { - fn drop(&mut self) { - if self.cleanup { - fs::remove_file(&self.path).ok(); - } - } -} - -impl Editor { - /// Create a new editor. - pub fn new(path: impl AsRef) -> io::Result { - let path = path.as_ref(); - if path.try_exists()? { - let meta = fs::metadata(path)?; - if !meta.is_file() { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "must be used to edit a file", - )); - } - } - Ok(Self { - path: path.to_path_buf(), - truncate: false, - cleanup: false, - }) - } +#[derive(Debug, Error)] +#[error(transparent)] +pub struct Error(#[from] InquireError); +impl Editor<'_> { + /// Create a new editor for editing a comment. pub fn comment() -> Self { - let path = env::temp_dir().join(COMMENT_FILE); - - Self { - path, - truncate: true, - cleanup: true, - } - } - - /// Set the file extension. - pub fn extension(mut self, ext: &str) -> Self { - let ext = ext.trim_start_matches('.'); - - self.path.set_extension(ext); - self - } - - /// Truncate the file to length 0 when opening - pub fn truncate(mut self, truncate: bool) -> Self { - self.truncate = truncate; - self - } - - /// Clean up the file after the [`Editor`] is dropped. - pub fn cleanup(mut self, cleanup: bool) -> Self { - self.cleanup = cleanup; - self - } - - /// Initialize the file with the provided `content`, as long as the file - /// does not already contain anything. - #[allow(clippy::byte_char_slices)] - pub fn initial(self, content: impl AsRef<[u8]>) -> io::Result { - let content = content.as_ref(); - let mut file = fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(self.truncate) - .open(&self.path)?; - - if file.metadata()?.len() == 0 { - file.write_all(content)?; - if !content.ends_with(&[b'\n']) { - file.write_all(b"\n")?; - } - file.flush()?; - } - Ok(self) + Self::new("Enter comment.", "You may enter your comment using your editor. Pressing (e) will open the editor. Save the file and exit the editor to submit your comment.") } /// Open the editor and return the edited text. /// /// If the text hasn't changed from the initial contents of the editor, /// return `None`. - pub fn edit(&mut self) -> io::Result> { - let Some(cmd) = self::default_editor() else { - return Err(io::Error::new( - io::ErrorKind::NotFound, - "editor not configured: the `EDITOR` environment variable is not set", - )); - }; - let Some(parts) = shlex::split(cmd.to_string_lossy().as_ref()) else { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("invalid editor command {cmd:?}"), - )); - }; - let Some((program, args)) = parts.split_first() else { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("invalid editor command {cmd:?}"), - )); - }; - - // We duplicate the stderr file descriptor to pass it to the child process, otherwise, if - // we simply pass the `RawFd` of our stderr, `Command` will close our stderr when the - // child exits. - let stderr = io::stderr().as_raw_fd(); - let stderr = unsafe { libc::dup(stderr) }; - let stdin = if io::stdin().is_terminal() { - process::Stdio::inherit() - } else if cfg!(unix) { - // If standard input is not a terminal device, the editor won't work correctly. - // In that case, we use the terminal device, eg. `/dev/tty` as standard input. - let tty = fs::OpenOptions::new() - .read(true) - .write(true) - .open("/dev/tty")?; - process::Stdio::from(tty) - } else { - return Err(io::Error::new( - io::ErrorKind::Unsupported, - format!("standard input is not a terminal, refusing to execute editor {cmd:?}"), - )); - }; - - process::Command::new(program) - .stdout(unsafe { process::Stdio::from_raw_fd(stderr) }) - .stderr(process::Stdio::inherit()) - .stdin(stdin) - .args(args) - .arg(&self.path) - .spawn() - .map_err(|e| { - io::Error::new( - e.kind(), - format!("failed to spawn editor command {cmd:?}: {e}"), - ) - })? - .wait() - .map_err(|e| { - io::Error::new( - e.kind(), - format!("editor command {cmd:?} didn't spawn: {e}"), - ) - })?; - - let text = fs::read_to_string(&self.path)?; - if text.trim().is_empty() { - return Ok(None); + pub fn edit(self) -> Result, Error> { + match self.0.prompt() { + Err(InquireError::OperationCanceled | InquireError::OperationInterrupted) => Ok(None), + Err(e) => Err(Error::from(e)), + Ok(s) => Ok(Some(s)), + } + } +} + +impl<'a> Editor<'a> { + /// Create a new editor. + pub fn new(message: &'a str, help_message: &'a str) -> Self { + Self(inquire::Editor::new(message).with_help_message(help_message)) + } + + /// Set the file extension. + pub fn extension(self, ext: &'a str) -> Self { + debug_assert!( + ext.starts_with('.'), + "File extension should start with a dot." + ); + Self(self.0.with_file_extension(ext)) + } + + /// Initialize the file with the provided `content`, as long as the file + /// does not already contain anything. + pub fn initial(self, content: &'a str) -> Self { + Self(self.0.with_predefined_text(content)) + } + + pub fn editor(self, editor: Option<&'a OsString>) -> Self { + match editor { + Some(editor_command) => Self(self.0.with_editor_command(editor_command)), + None => self, } - Ok(Some(text)) } } /// Get the default editor command. -fn default_editor() -> Option { +pub fn default_editor_command() -> Option { // First check the standard environment variables. if let Ok(visual) = env::var("VISUAL") { if !visual.is_empty() { @@ -211,6 +94,9 @@ fn default_editor() -> Option { /// We don't bother checking the $PATH variable, as we're only looking for very standard tools /// and prefer not to make this too complex. fn exists(cmd: &str) -> bool { + // Some common paths where system-installed binaries are found. + const PATHS: &[&str] = &["/usr/local/bin", "/usr/bin", "/bin"]; + for dir in PATHS { if Path::new(dir).join(cmd).exists() { return true;