term: Revert using inquire to spawn editor

This is morally a revert of 70fb0d3fef
while additionally getting to compile
`crates/radicle-term/src/editor.rs` on Windows.

To get the changes that were made on top of the revert, check

  git diff 70fb0d3fef~1 <this commit> -- crates/radicle-term/src/editor.rs

There were reports of the inquire editor being unrealiable.
This commit is contained in:
Lorenz Leutgeb 2025-08-05 13:19:03 +02:00
parent 4934473b86
commit 5d467418be
8 changed files with 206 additions and 86 deletions

2
Cargo.lock generated
View File

@ -2793,8 +2793,10 @@ dependencies = [
"crossterm 0.29.0",
"git2",
"inquire",
"libc",
"pretty_assertions",
"radicle-signals",
"shlex",
"tempfile",
"thiserror 1.0.69",
"unicode-display-width",

View File

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

View File

@ -411,9 +411,8 @@ 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()
.editor(term::editor::default_editor_command().as_ref())
.extension(".json")
.initial(&serde_json::to_string_pretty(&current.doc)?)
.extension("json")
.initial(serde_json::to_string_pretty(&current.doc)?)?
.edit()?
{
Some(proposal) => serde_json::from_str::<RawDoc>(&proposal)?,

View File

@ -836,8 +836,6 @@ 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)]
@ -862,9 +860,8 @@ impl CommentBuilder {
writeln!(&mut input, "> {line}")?;
}
let output = term::Editor::comment()
.editor(term::editor::default_editor_command().as_ref())
.extension(".diff")
.initial(&input)
.extension("diff")
.initial(input)?
.edit()?;
if let Some(output) = output {

View File

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

View File

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

View File

@ -22,9 +22,11 @@ unicode-display-width = "0.3.0"
unicode-segmentation = "1.7.1"
zeroize = { workspace = true }
git2 = { workspace = true, features = ["vendored-libgit2"], optional = true }
shlex = { workspace = true }
[target.'cfg(unix)'.dependencies]
crossbeam-channel = { workspace = true }
libc = { workspace = true }
radicle-signals = { workspace = true }
[dev-dependencies]

View File

@ -1,69 +1,197 @@
use std::env;
use std::ffi::OsString;
use std::path::Path;
use inquire::InquireError;
use thiserror::Error;
use std::io::IsTerminal;
use std::io::Write;
use std::path::{Path, PathBuf};
use std::process;
use std::{env, fs, io};
/// Allows for text input in the configured editor.
pub struct Editor<'a>(pub inquire::Editor<'a>);
pub struct Editor {
path: PathBuf,
truncate: bool,
cleanup: bool,
}
#[derive(Debug, Error)]
#[error(transparent)]
pub struct Error(#[from] InquireError);
impl Default for Editor {
fn default() -> Self {
Self::comment()
}
}
impl<'a> Editor<'a> {
pub const HELP: &'a str = "(e) to edit. (enter) to save and exit. (esc) to cancel and quit.";
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,
})
}
/// Create a new editor for editing a comment.
pub fn comment() -> Self {
Self::new("Enter comment.", Self::HELP)
const COMMENT_FILE: &str = "RAD_COMMENT";
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<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.
///
/// If the text hasn't changed from the initial contents of the editor,
/// return `None`.
pub fn edit(self) -> Result<Option<String>, 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,
pub fn edit(&mut self) -> io::Result<Option<String>> {
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:?}"),
));
};
let stdout: process::Stdio = {
#[cfg(unix)]
{
use std::os::fd::{AsRawFd as _, FromRawFd as _};
// 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();
unsafe { process::Stdio::from_raw_fd(libc::dup(stderr)) }
}
#[cfg(not(unix))]
{
// No duplication of the file handle for stderr on Windows.
// This might not always work, but is better than not being able to build for
// Windows.
io::stderr().into()
}
};
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(stdout)
.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.
pub fn default_editor_command() -> Option<OsString> {
fn default_editor() -> Option<OsString> {
// First check the standard environment variables.
if let Ok(visual) = env::var("VISUAL") {
if !visual.is_empty() {