term: Use inquire for spawning the editor

`radicle-term` includes quite a lot of rather "low-level" logic to spawn
the preferred editor of the user. In particular, this logic is
platform-dependent and only works on Unix-like platforms.

Also, `radicle-term` already depends on `inquire` which features an
editor prompt. It is a cross-platform solution to spawn the editor.
This commit changes the implementation of `Editor` to be a wrapper of
`inquire::Editor`, keeping the interface mostly intact.

Downsides:
 - We cannot edit a file "in place" this way, instead we have to read
   it, as `inquire` only supports editing temporary files. This can also
   be viewed as a benefit, as we lessen the risk of corruption of these
   files.
 - `inquire::Editor` contains borrows, so it is quite cumbersome to
   extend it with a custom mechanism for looking up the preferred
   editor. The lookup logic was kept, but needs to be invoked by the
   caller. In the future we might consider just using the logic provided
   by `inquire` or requesting a change to `inquire`.
 - The prompt is a bit strange. It does not show the current state of
   the output, see <https://github.com/mikaelmello/inquire/issues/280>
   and <https://github.com/mikaelmello/inquire/pull/271>.
This commit is contained in:
Lorenz Leutgeb 2025-07-15 16:40:52 +02:00
parent a28fd65e89
commit 70fb0d3fef
6 changed files with 86 additions and 189 deletions

View File

@ -185,12 +185,23 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
path.display() path.display()
); );
} }
Operation::Edit => match term::editor::Editor::new(&path)?.extension("json").edit()? { Operation::Edit => {
Some(_) => { let config = std::fs::read_to_string(&path)?;
term::success!("Successfully made changes to the configuration at {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(()) Ok(())

View File

@ -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. // If `--edit` is specified, the document can also be edited via a text edit.
let proposal = if edit { let proposal = if edit {
match term::editor::Editor::comment() match term::editor::Editor::comment()
.extension("json") .editor(term::editor::default_editor_command().as_ref())
.initial(serde_json::to_string_pretty(&current.doc)?)? .extension(".json")
.initial(&serde_json::to_string_pretty(&current.doc)?)
.edit()? .edit()?
{ {
Some(proposal) => serde_json::from_str::<RawDoc>(&proposal)?, Some(proposal) => serde_json::from_str::<RawDoc>(&proposal)?,

View File

@ -836,6 +836,8 @@ enum Error {
Format(#[from] std::fmt::Error), Format(#[from] std::fmt::Error),
#[error(transparent)] #[error(transparent)]
Git(#[from] git::raw::Error), Git(#[from] git::raw::Error),
#[error("editor error: {0}")]
Editor(#[from] term::editor::Error),
} }
#[derive(Debug)] #[derive(Debug)]
@ -860,8 +862,9 @@ impl CommentBuilder {
writeln!(&mut input, "> {line}")?; writeln!(&mut input, "> {line}")?;
} }
let output = term::Editor::comment() let output = term::Editor::comment()
.extension("diff") .editor(term::editor::default_editor_command().as_ref())
.initial(input)? .extension(".diff")
.initial(&input)
.edit()?; .edit()?;
if let Some(output) = output { if let Some(output) = output {

View File

@ -1,5 +1,3 @@
use std::io;
use radicle_term::table::TableOptions; use radicle_term::table::TableOptions;
use radicle_term::{Table, VStack}; use radicle_term::{Table, VStack};
@ -34,7 +32,7 @@ pub enum Format {
pub fn get_title_description( pub fn get_title_description(
title: Option<String>, title: Option<String>,
description: Option<String>, description: Option<String>,
) -> io::Result<Option<(String, String)>> { ) -> Result<Option<(String, String)>, term::patch::Error> {
term::patch::Message::edit_title_description(title, description, OPEN_MSG) term::patch::Message::edit_title_description(title, description, OPEN_MSG)
} }

View File

@ -31,6 +31,10 @@ pub enum Error {
Io(#[from] io::Error), Io(#[from] io::Error),
#[error("invalid utf-8 string")] #[error("invalid utf-8 string")]
InvalidUtf8, InvalidUtf8,
#[error("editor error: {0}")]
Editor(#[from] term::editor::Error),
#[error("a patch title must be provided")]
PatchTitleMissing,
} }
/// The user supplied `Patch` description. /// The user supplied `Patch` description.
@ -47,13 +51,14 @@ pub enum Message {
impl Message { impl Message {
/// Get the `Message` as a string according to the method. /// Get the `Message` as a string according to the method.
pub fn get(self, help: &str) -> std::io::Result<String> { pub fn get(self, help: &str) -> Result<String, term::editor::Error> {
let comment = match self { let comment = match self {
Message::Edit => { Message::Edit => {
if io::stderr().is_terminal() { if io::stderr().is_terminal() {
term::Editor::comment() term::Editor::comment()
.extension("markdown") .editor(term::editor::default_editor_command().as_ref())
.initial(help)? .extension(".md")
.initial(help)
.edit()? .edit()?
} else { } else {
Some(help.to_owned()) Some(help.to_owned())
@ -75,7 +80,7 @@ impl Message {
title: Option<String>, title: Option<String>,
description: Option<String>, description: Option<String>,
help: &str, help: &str,
) -> std::io::Result<Option<(String, String)>> { ) -> Result<Option<(String, String)>, Error> {
let mut placeholder = String::new(); let mut placeholder = String::new();
if let Some(title) = title { 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()); let (title, description) = (title.trim().to_string(), description.trim().to_string());
if title.is_empty() { if title.is_empty() {
return Err(io::Error::new( return Err(Error::PatchTitleMissing);
io::ErrorKind::InvalidInput,
"a patch title must be provided",
)
.into());
} }
Ok((title, description)) Ok((title, description))
@ -249,7 +250,7 @@ fn edit_display_message(title: &str, description: &str) -> String {
pub fn get_edit_message( pub fn get_edit_message(
patch_message: term::patch::Message, patch_message: term::patch::Message,
patch: &cob::patch::Patch, patch: &cob::patch::Patch,
) -> io::Result<(String, String)> { ) -> Result<(String, String), Error> {
let display_msg = edit_display_message(patch.title(), patch.description()); let display_msg = edit_display_message(patch.title(), patch.description());
let patch_message = patch_message.get(&display_msg)?; let patch_message = patch_message.get(&display_msg)?;
let patch_message = patch_message.replace(PATCH_MSG.trim(), ""); // Delete help message. 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()); let (title, description) = (title.trim().to_string(), description.trim().to_string());
if title.is_empty() { if title.is_empty() {
return Err(io::Error::new( return Err(Error::PatchTitleMissing);
io::ErrorKind::InvalidInput,
"a patch title must be provided",
));
} }
Ok((title, description)) Ok((title, description))

View File

@ -1,184 +1,67 @@
use std::env;
use std::ffi::OsString; use std::ffi::OsString;
use std::io::IsTerminal; use std::path::Path;
use std::io::Write;
use std::os::fd::{AsRawFd, FromRawFd};
use std::path::{Path, PathBuf};
use std::process;
use std::{env, fs, io};
pub const COMMENT_FILE: &str = "RAD_COMMENT"; use inquire::InquireError;
/// Some common paths where system-installed binaries are found. use thiserror::Error;
pub const PATHS: &[&str] = &["/usr/local/bin", "/usr/bin", "/bin"];
/// Allows for text input in the configured editor. /// Allows for text input in the configured editor.
pub struct Editor { pub struct Editor<'a>(pub inquire::Editor<'a>);
path: PathBuf,
truncate: bool,
cleanup: bool,
}
impl Default for Editor { #[derive(Debug, Error)]
fn default() -> Self { #[error(transparent)]
Self::comment() pub struct Error(#[from] InquireError);
}
}
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<Path>) -> io::Result<Self> {
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,
})
}
impl Editor<'_> {
/// Create a new editor for editing a comment.
pub fn comment() -> Self { pub fn comment() -> Self {
let path = env::temp_dir().join(COMMENT_FILE); 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.")
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<Self> {
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)
} }
/// Open the editor and return the edited text. /// Open the editor and return the edited text.
/// ///
/// If the text hasn't changed from the initial contents of the editor, /// If the text hasn't changed from the initial contents of the editor,
/// return `None`. /// return `None`.
pub fn edit(&mut self) -> io::Result<Option<String>> { pub fn edit(self) -> Result<Option<String>, Error> {
let Some(cmd) = self::default_editor() else { match self.0.prompt() {
return Err(io::Error::new( Err(InquireError::OperationCanceled | InquireError::OperationInterrupted) => Ok(None),
io::ErrorKind::NotFound, Err(e) => Err(Error::from(e)),
"editor not configured: the `EDITOR` environment variable is not set", Ok(s) => Ok(Some(s)),
)); }
}; }
let Some(parts) = shlex::split(cmd.to_string_lossy().as_ref()) else { }
return Err(io::Error::new(
io::ErrorKind::InvalidInput, impl<'a> Editor<'a> {
format!("invalid editor command {cmd:?}"), /// 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))
let Some((program, args)) = parts.split_first() else { }
return Err(io::Error::new(
io::ErrorKind::InvalidInput, /// Set the file extension.
format!("invalid editor command {cmd:?}"), pub fn extension(self, ext: &'a str) -> Self {
)); debug_assert!(
}; ext.starts_with('.'),
"File extension should start with a dot."
// 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 Self(self.0.with_file_extension(ext))
// child exits. }
let stderr = io::stderr().as_raw_fd();
let stderr = unsafe { libc::dup(stderr) }; /// Initialize the file with the provided `content`, as long as the file
let stdin = if io::stdin().is_terminal() { /// does not already contain anything.
process::Stdio::inherit() pub fn initial(self, content: &'a str) -> Self {
} else if cfg!(unix) { Self(self.0.with_predefined_text(content))
// 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() pub fn editor(self, editor: Option<&'a OsString>) -> Self {
.read(true) match editor {
.write(true) Some(editor_command) => Self(self.0.with_editor_command(editor_command)),
.open("/dev/tty")?; None => self,
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);
} }
Ok(Some(text))
} }
} }
/// Get the default editor command. /// Get the default editor command.
fn default_editor() -> Option<OsString> { pub fn default_editor_command() -> Option<OsString> {
// First check the standard environment variables. // First check the standard environment variables.
if let Ok(visual) = env::var("VISUAL") { if let Ok(visual) = env::var("VISUAL") {
if !visual.is_empty() { if !visual.is_empty() {
@ -211,6 +94,9 @@ fn default_editor() -> Option<OsString> {
/// We don't bother checking the $PATH variable, as we're only looking for very standard tools /// 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. /// and prefer not to make this too complex.
fn exists(cmd: &str) -> bool { 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 { for dir in PATHS {
if Path::new(dir).join(cmd).exists() { if Path::new(dir).join(cmd).exists() {
return true; return true;