src/body.rs
Ref: Size: 8.0 KiB History
//! Getting prose into git-collab.
//!
//! A body used to arrive exactly one way: as a shell argument. That is a bad
//! door for the kind of text this tool carries. Review prose is long,
//! multi-line, and full of things a shell wants to interpret — backticks for
//! code spans, `$` in a variable name being discussed, quotes of both kinds,
//! and `?` or `*` in ordinary sentences. A reviewer on a hardened shell had a
//! comment rejected outright for containing a `?` and had to re-author it
//! around the shell. Prose should never have to be written around the tool
//! that stores it.
//!
//! So every command taking `--body` also takes `--body-file <path>`, with `-`
//! meaning stdin, under the short flag `-F`. That is git's own convention
//! (`git commit -F`), which is the point: users already know it, and one
//! convention covering the whole tool is worth more than a bespoke one
//! covering the three commands an issue happened to name.
//!
//! Nothing here trims, strips, or normalizes. What the caller supplies is
//! what gets recorded, byte for byte, including trailing newlines, CRLF and
//! non-ASCII. Bodies are markdown and go into an append-only DAG: a tidy-up
//! applied on the way in is a corruption nobody can correct afterwards, and
//! one that would only surface in someone's review three weeks later.
use std::io::{IsTerminal, Read};
use crate::editor;
use crate::error::Error;
/// How a command was asked to find its body.
pub struct BodyArgs<'a> {
pub body: Option<&'a str>,
pub body_file: Option<&'a str>,
}
impl<'a> BodyArgs<'a> {
pub fn new(body: Option<&'a str>, body_file: Option<&'a str>) -> Self {
BodyArgs { body, body_file }
}
/// Read whichever of the two was supplied, rejecting both at once.
fn read_supplied(&self) -> Result<Option<String>, Error> {
match (self.body, self.body_file) {
(Some(_), Some(_)) => Err(Error::Cmd(
"--body and --body-file are mutually exclusive; pass one or the other".to_string(),
)),
(Some(text), None) => Ok(Some(text.to_string())),
(None, Some(path)) => read_file_or_stdin(path).map(Some),
(None, None) => Ok(None),
}
}
}
/// Read a body from a path, or from stdin when the path is `-`.
fn read_file_or_stdin(path: &str) -> Result<String, Error> {
let bytes = if path == "-" {
let mut buf = Vec::new();
std::io::stdin()
.read_to_end(&mut buf)
.map_err(|e| Error::Cmd(format!("failed to read the body from stdin: {}", e)))?;
buf
} else {
std::fs::read(path)
.map_err(|e| Error::Cmd(format!("failed to read the body from {}: {}", path, e)))?
};
String::from_utf8(bytes).map_err(|_| {
Error::Cmd(format!(
"the body read from {} is not valid UTF-8",
if path == "-" { "stdin" } else { path }
))
})
}
/// The error a non-interactive caller gets when it supplies no body at all.
///
/// Naming the alternatives matters more here than anywhere else in the tool:
/// the callers that hit this are scripts and agents, which have no terminal
/// to open an editor on and no human to read a terse refusal.
fn no_body_error(what: &str) -> Error {
Error::Cmd(format!(
"no {what} given: pass --body <text>, --body-file <path>, or --body-file - to read stdin.\n\
(An editor is opened instead only when stdin is a terminal and $EDITOR or $VISUAL is set.)"
))
}
/// Reject a body that is entirely blank.
///
/// An empty body is almost always an accident — an editor closed without
/// saving, an empty file passed by a script — and an accident recorded in an
/// append-only DAG is permanent. Only whitespace-only bodies are refused;
/// anything with content in it is passed through untouched, trailing blank
/// lines and all.
fn reject_if_blank(body: String, what: &str) -> Result<String, Error> {
if body.trim().is_empty() {
return Err(Error::Cmd(format!("empty {what}; aborting")));
}
Ok(body)
}
/// Whether an editor can be opened right now: there has to be one configured,
/// and stdin has to be a terminal.
///
/// The terminal check is what keeps this safe for scripts and agents. An
/// editor launched with no terminal does not fail — it blocks, forever, on a
/// command the caller expected to return. A clear error beats a hang.
fn can_open_editor() -> bool {
editor::resolve_editor().is_some() && std::io::stdin().is_terminal()
}
/// Resolve a body that the command requires, opening an editor seeded with
/// `initial` when none was supplied and a terminal is available.
///
/// `what` names the thing being written ("comment", "review body") so the
/// error reads as a sentence.
pub fn resolve_required(args: &BodyArgs, initial: &str, what: &str) -> Result<String, Error> {
if let Some(body) = args.read_supplied()? {
return reject_if_blank(body, what);
}
if !can_open_editor() {
return Err(no_body_error(what));
}
reject_if_blank(editor::compose(initial)?, what)
}
/// Resolve a body that the command treats as optional.
///
/// No body means "leave it as it is" (`issue edit`) or "there isn't one"
/// (`issue open`), which are both meaningful answers — so unlike
/// [`resolve_required`], omitting it never opens an editor. Doing otherwise
/// would turn every existing non-interactive `issue open -t ...` into a
/// command that blocks on an editor.
pub fn resolve_optional(args: &BodyArgs) -> Result<Option<String>, Error> {
args.read_supplied()
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn body_flag_is_passed_through_verbatim() {
let args = BodyArgs::new(Some(" spaced\n\n"), None);
assert_eq!(
resolve_required(&args, "", "comment").unwrap(),
" spaced\n\n"
);
}
#[test]
fn both_options_together_are_rejected() {
let args = BodyArgs::new(Some("a"), Some("b"));
let err = format!("{}", resolve_required(&args, "", "comment").unwrap_err());
assert!(
err.contains("--body") && err.contains("--body-file"),
"{}",
err
);
}
#[test]
fn a_file_is_read_byte_for_byte() {
let mut file = tempfile::NamedTempFile::new().unwrap();
let content = "trailing newlines\n\n\nnaïve 🎉\r\n";
std::io::Write::write_all(&mut file, content.as_bytes()).unwrap();
let path = file.path().to_str().unwrap().to_string();
let args = BodyArgs::new(None, Some(&path));
assert_eq!(resolve_required(&args, "", "comment").unwrap(), content);
}
#[test]
fn a_missing_file_names_the_path() {
let args = BodyArgs::new(None, Some("/nonexistent/git-collab-body.txt"));
let err = format!("{}", resolve_required(&args, "", "comment").unwrap_err());
assert!(err.contains("/nonexistent/git-collab-body.txt"), "{}", err);
}
#[test]
fn a_blank_body_is_refused() {
let args = BodyArgs::new(Some(" \n\t\n"), None);
let err = format!("{}", resolve_required(&args, "", "comment").unwrap_err());
assert!(err.contains("empty comment"), "{}", err);
}
#[test]
fn invalid_utf8_is_refused_rather_than_mangled() {
let mut file = tempfile::NamedTempFile::new().unwrap();
std::io::Write::write_all(&mut file, &[0xff, 0xfe, 0x00]).unwrap();
let path = file.path().to_str().unwrap().to_string();
let args = BodyArgs::new(None, Some(&path));
let err = format!("{}", resolve_required(&args, "", "comment").unwrap_err());
assert!(err.contains("not valid UTF-8"), "{}", err);
}
#[test]
fn an_optional_body_stays_absent_when_nothing_is_given() {
let args = BodyArgs::new(None, None);
assert_eq!(resolve_optional(&args).unwrap(), None);
}
#[test]
fn an_optional_body_may_be_blank() {
// `issue open -b ""` has always meant "no description", and still does.
let args = BodyArgs::new(Some(""), None);
assert_eq!(resolve_optional(&args).unwrap(), Some(String::new()));
}
}