From c490bd5a276c2bd03bf7919f817859373da452b5 Mon Sep 17 00:00:00 2001 From: Bowen Ho Date: Sat, 23 May 2026 23:42:03 +0800 Subject: [PATCH] refactor: split composer config into subparts compose, reply and forward * feat: Add configs for `reply-with` and `forward-with` commands Config extended from: (Old) ```toml [message.composer.mml] command = "mml compose" ``` , where `compose-with`, `reply-with`, and `forward-with` all share the same composer command, to: (New) ```toml [message.composer.mml] default = true compose-command = "mml compose" reply-command = "mml reply" forward-command = "mml forward" ``` * docs: ComposerConfig * refactor(account): simplify composer resolution with `get_composer` - Implement `Account::get_composer`, and `Account::get_reader` to fetch config by name or default. - Remove the redundant `resolve_composer`, `default_composer` `resolve_reader`, and `default_reader` helpers. - Simplify the configuration retrieval architecture. * refactor: update composer and reader config to use std::process::Command - Remove `_command` suffix from `compose`, `reply`, and `forward` configuration fields. - Replace composer and reader config command type from `String` to `std::process::Command`. - Remove `Clone` derives from `Config`, `Account`, and related structs due to `Command` type limitations. Refs: #687 --- src/account/context.rs | 37 ++++++++++++++- src/config.rs | 49 +++++++++++++++----- src/shared/messages/compose_with.rs | 18 ++++---- src/shared/messages/forward_with.rs | 18 ++++---- src/shared/messages/mailto.rs | 18 ++++---- src/shared/messages/read_with.rs | 13 +++--- src/shared/messages/reply_with.rs | 15 +++--- src/shared/messages/runner.rs | 71 ++++------------------------- 8 files changed, 126 insertions(+), 113 deletions(-) diff --git a/src/account/context.rs b/src/account/context.rs index dc096f56..3970cf02 100644 --- a/src/account/context.rs +++ b/src/account/context.rs @@ -30,6 +30,7 @@ use std::{collections::HashMap, env::temp_dir, path::PathBuf}; +use anyhow::{Result, anyhow}; use comfy_table::{Color as TableColor, ContentArrangement, presets}; use crossterm::style::Color; use dirs::download_dir; @@ -48,7 +49,7 @@ const DEFAULT_REPLIED_CHAR: char = 'R'; const DEFAULT_FLAGGED_CHAR: char = '!'; const DEFAULT_ATTACHMENT_CHAR: char = '@'; -#[derive(Clone, Debug, Default)] +#[derive(Debug, Default)] pub struct Account { pub downloads_dir: Option, pub table_preset: Option, @@ -197,6 +198,40 @@ impl Account { .map(String::as_str) } + /// Gets a composer configuration. When `name` is given, looks up + /// the corresponding entry and bails if missing. + /// When `name` is `None`, returns the entry with `default = true`, + /// or bails with a hint if no default is set. + pub fn get_composer_mut(&mut self, name: Option<&str>) -> Result<&mut ComposerConfig> { + match name { + Some(name) => self + .composer + .get_mut(name) + .ok_or(anyhow!("no composer named `{name}` in [message.composer]")), + None => self + .composer + .values_mut() + .find(|c| c.default) + .ok_or(anyhow!( + "no composer specified and no default in [message.composer.*]; \ + pass a or set `default = true` on one entry" + )), + } + } + + pub fn get_reader_mut(&mut self, name: Option<&str>) -> Result<&mut ReaderConfig> { + match name { + Some(name) => self + .reader + .get_mut(name) + .ok_or(anyhow!("no reader named `{name}` in [message.reader]")), + None => self.reader.values_mut().find(|c| c.default).ok_or(anyhow!( + "no reader specified and no default in [message.reader.*]; \ + pass a or set `default = true` on one entry" + )), + } + } + // ── envelopes list — flag glyphs ───────────────────────────────────── pub fn envelopes_list_table_unseen_char(&self) -> char { diff --git a/src/config.rs b/src/config.rs index 05a25086..9043d4c0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -15,12 +15,13 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -use std::{collections::HashMap, fs, path::Path, path::PathBuf}; +use std::{collections::HashMap, fs, path::Path, path::PathBuf, process::Command}; use anyhow::{Context, Result}; use comfy_table::ContentArrangement; use crossterm::style::Color; use pimalaya_config::{ + command, secret::Secret, toml::{TomlConfig, shell_expanded_string}, }; @@ -39,7 +40,7 @@ use serde::{Deserialize, Serialize}; /// file can be shared with `himalaya-tui`: top-level TUI-only fields /// (`display-name`, `signature`, `signature-delim`) are silently /// ignored here. -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Debug, Default, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] pub struct Config { pub downloads_dir: Option, @@ -290,7 +291,7 @@ pub struct EnvelopeListTableConfig { /// stdin and emit human-readable bytes on stdout (called by /// `read-with`). Both are looked up by name; the entry flagged /// `default = true` is used when no name is passed. -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Debug, Default, Deserialize, Serialize)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] pub struct MessageConfig { #[serde(default)] @@ -300,14 +301,39 @@ pub struct MessageConfig { } /// Single composer entry under `[message.composer.]`. -#[derive(Clone, Debug, Deserialize, Serialize)] +/// +/// For all shell command strings defined below: +/// - The command is invoked via `sh -c`. +/// - stdin behavior varies by command as documented below. +/// - stdout is captured as the MIME draft. +/// - stderr is inherited so the composer can prompt the user. +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] pub struct ComposerConfig { - /// Shell command line invoked via `sh -c`. Stdin carries the - /// source MIME bytes (empty for new messages); stdout is - /// captured as the MIME draft; stderr is inherited so the - /// composer can prompt the user. - pub command: String, + /// Command used to write a brand new message. + /// + /// This is invoked by the `compose-with` and `mailto` commands. + /// + /// - When invoked by `compose-with`, stdin is empty. + /// - When invoked by `mailto`, stdin is piped with a pre-filled RFC 5322 + /// draft skeleton built from the parsed RFC 6068 `mailto:` URI parameters + /// (such as to, cc, bcc, subject, and body). + #[serde(with = "command")] + pub compose: Command, + + /// Command used to reply to an existing message. + /// + /// This is invoked by the `reply-with` command. The original message's + /// MIME bytes are passed via stdin. + #[serde(with = "command")] + pub reply: Command, + + /// Command used to forward an existing message. + /// + /// This is invoked by the `forward-with` command. The original message's + /// MIME bytes are passed via stdin. + #[serde(with = "command")] + pub forward: Command, /// Marks this entry as the fallback when `compose-with` / /// `reply-with` / `forward-with` are invoked without a name. @@ -318,14 +344,15 @@ pub struct ComposerConfig { } /// Single reader entry under `[message.reader.]`. -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] pub struct ReaderConfig { /// Shell command line invoked via `sh -c`. Stdin carries the /// source MIME bytes; stdout is forwarded to the terminal (zero /// bytes is fine — the reader may have spawned its own UI); /// stderr is inherited. - pub command: String, + #[serde(with = "command")] + pub command: Command, /// Marks this entry as the fallback when `read-with` is /// invoked without a name. diff --git a/src/shared/messages/compose_with.rs b/src/shared/messages/compose_with.rs index eaa4e24d..a4767235 100644 --- a/src/shared/messages/compose_with.rs +++ b/src/shared/messages/compose_with.rs @@ -18,6 +18,7 @@ use anyhow::{Result, bail}; use clap::Parser; use pimalaya_cli::printer::Printer; +use pimalaya_config::command::shell; use crate::shared::{ client::EmailClient, @@ -57,16 +58,17 @@ pub struct MessageComposeWithCommand { impl MessageComposeWithCommand { pub fn execute(self, printer: &mut impl Printer, mut client: EmailClient) -> Result<()> { - let command = match self.command.as_deref() { - Some(cmd) => cmd.to_owned(), - None => { - runner::resolve_composer(&client.account.composer, self.name.as_deref())?.to_owned() - } - }; + let mut command = self.command.map(|cmd| shell(&cmd)); + let command = command.as_mut().unwrap_or( + &mut client + .account + .get_composer_mut(self.name.as_deref())? + .compose, + ); - let raw = runner::run(&command, &[])?; + let raw = runner::run(command, &[])?; if raw.is_empty() { - bail!("composer `{command}` produced no output"); + bail!("composer `{command:?}` produced no output"); } output::route(printer, &mut client, raw, self.save.as_deref(), self.send) diff --git a/src/shared/messages/forward_with.rs b/src/shared/messages/forward_with.rs index f2f13f67..721ce410 100644 --- a/src/shared/messages/forward_with.rs +++ b/src/shared/messages/forward_with.rs @@ -18,6 +18,7 @@ use anyhow::{Result, bail}; use clap::Parser; use pimalaya_cli::printer::Printer; +use pimalaya_config::command::shell; use crate::shared::{ client::EmailClient, @@ -59,16 +60,17 @@ impl MessageForwardWithCommand { pub fn execute(self, printer: &mut impl Printer, mut client: EmailClient) -> Result<()> { let source = client.get_message(&self.mailbox, &self.id)?; - let command = match self.command.as_deref() { - Some(cmd) => cmd.to_owned(), - None => { - runner::resolve_composer(&client.account.composer, self.name.as_deref())?.to_owned() - } - }; + let mut command = self.command.map(|cmd| shell(&cmd)); + let command = command.as_mut().unwrap_or( + &mut client + .account + .get_composer_mut(self.name.as_deref())? + .forward, + ); - let raw = runner::run(&command, &source)?; + let raw = runner::run(command, &source)?; if raw.is_empty() { - bail!("composer `{command}` produced no output"); + bail!("composer `{command:?}` produced no output"); } output::route(printer, &mut client, raw, self.save.as_deref(), self.send) diff --git a/src/shared/messages/mailto.rs b/src/shared/messages/mailto.rs index addfaaf5..a0583eca 100644 --- a/src/shared/messages/mailto.rs +++ b/src/shared/messages/mailto.rs @@ -19,6 +19,7 @@ use anyhow::{Result, anyhow, bail}; use clap::Parser; use percent_encoding::percent_decode_str; use pimalaya_cli::printer::Printer; +use pimalaya_config::command::shell; use url::Url; use crate::shared::{ @@ -88,16 +89,17 @@ impl MessageMailtoCommand { None, )?; - let command = match self.command.as_deref() { - Some(cmd) => cmd.to_owned(), - None => { - runner::resolve_composer(&client.account.composer, self.name.as_deref())?.to_owned() - } - }; + let mut command = self.command.map(|cmd| shell(&cmd)); + let command = command.as_mut().unwrap_or( + &mut client + .account + .get_composer_mut(self.name.as_deref())? + .compose, + ); - let raw = runner::run(&command, &draft)?; + let raw = runner::run(command, &draft)?; if raw.is_empty() { - bail!("composer `{command}` produced no output"); + bail!("composer `{command:?}` produced no output"); } output::route(printer, &mut client, raw, self.save.as_deref(), self.send) diff --git a/src/shared/messages/read_with.rs b/src/shared/messages/read_with.rs index eca3e82f..621463ad 100644 --- a/src/shared/messages/read_with.rs +++ b/src/shared/messages/read_with.rs @@ -20,6 +20,7 @@ use std::io::{Write, stdout}; use anyhow::Result; use clap::Parser; use pimalaya_cli::printer::Printer; +use pimalaya_config::command::shell; use crate::shared::{client::EmailClient, messages::runner}; @@ -58,14 +59,12 @@ impl MessageReadWithCommand { pub fn execute(self, _printer: &mut impl Printer, mut client: EmailClient) -> Result<()> { let source = client.get_message(&self.mailbox, &self.id)?; - let command = match self.command.as_deref() { - Some(cmd) => cmd.to_owned(), - None => { - runner::resolve_reader(&client.account.reader, self.name.as_deref())?.to_owned() - } - }; + let mut command = self.command.map(|cmd| shell(&cmd)); + let command = command + .as_mut() + .unwrap_or(&mut client.account.get_reader_mut(self.name.as_deref())?.command); - let bytes = runner::run(&command, &source)?; + let bytes = runner::run(command, &source)?; if !bytes.is_empty() { let mut out = stdout().lock(); diff --git a/src/shared/messages/reply_with.rs b/src/shared/messages/reply_with.rs index 81ef4889..b4c9b1bb 100644 --- a/src/shared/messages/reply_with.rs +++ b/src/shared/messages/reply_with.rs @@ -18,6 +18,7 @@ use anyhow::{Result, bail}; use clap::Parser; use pimalaya_cli::printer::Printer; +use pimalaya_config::command::shell; use crate::shared::{ client::EmailClient, @@ -64,16 +65,14 @@ impl MessageReplyWithCommand { pub fn execute(self, printer: &mut impl Printer, mut client: EmailClient) -> Result<()> { let source = client.get_message(&self.mailbox, &self.id)?; - let command = match self.command.as_deref() { - Some(cmd) => cmd.to_owned(), - None => { - runner::resolve_composer(&client.account.composer, self.name.as_deref())?.to_owned() - } - }; + let mut command = self.command.map(|cmd| shell(&cmd)); + let command = command + .as_mut() + .unwrap_or(&mut client.account.get_composer_mut(self.name.as_deref())?.reply); - let raw = runner::run(&command, &source)?; + let raw = runner::run(command, &source)?; if raw.is_empty() { - bail!("composer `{command}` produced no output"); + bail!("composer `{command:?}` produced no output"); } output::route(printer, &mut client, raw, self.save.as_deref(), self.send) diff --git a/src/shared/messages/runner.rs b/src/shared/messages/runner.rs index de0690cb..4a15360f 100644 --- a/src/shared/messages/runner.rs +++ b/src/shared/messages/runner.rs @@ -26,90 +26,37 @@ //! `/dev/tty` once they've consumed stdin — standard Unix practice. use std::{ - collections::HashMap, io::Write, process::{Command, Stdio}, }; use anyhow::{Result, anyhow, bail}; -use crate::config::{ComposerConfig, ReaderConfig}; - -/// Resolves a composer entry to its shell command line. When `name` -/// is given, looks up the corresponding entry and bails if missing. -/// When `name` is `None`, returns the entry with `default = true`, -/// or bails with a hint if no default is set. -pub fn resolve_composer<'a>( - composers: &'a HashMap, - name: Option<&str>, -) -> Result<&'a str> { - match name { - Some(name) => match composers.get(name) { - Some(entry) => Ok(entry.command.as_str()), - None => bail!("no composer named `{name}` in [message.composer]"), - }, - None => default_composer(composers).map(|entry| entry.command.as_str()), - } -} - -/// Same as [`resolve_composer`] but for readers. -pub fn resolve_reader<'a>( - readers: &'a HashMap, - name: Option<&str>, -) -> Result<&'a str> { - match name { - Some(name) => match readers.get(name) { - Some(entry) => Ok(entry.command.as_str()), - None => bail!("no reader named `{name}` in [message.reader]"), - }, - None => default_reader(readers).map(|entry| entry.command.as_str()), - } -} - -fn default_composer(composers: &HashMap) -> Result<&ComposerConfig> { - composers.values().find(|c| c.default).ok_or_else(|| { - anyhow!( - "no composer specified and no default in [message.composer.*]; \ - pass a or set `default = true` on one entry" - ) - }) -} - -fn default_reader(readers: &HashMap) -> Result<&ReaderConfig> { - readers.values().find(|c| c.default).ok_or_else(|| { - anyhow!( - "no reader specified and no default in [message.reader.*]; \ - pass a or set `default = true` on one entry" - ) - }) -} - -/// Spawns `command` through `sh -c`, writes `stdin_bytes` to its -/// stdin, and returns the captured stdout bytes. Stderr is inherited. +/// Spawns `command`, writes `stdin_bytes` to its +/// stdin, and returns the captured stdout bytes. +/// Stderr is inherited. /// Bails on a non-zero exit status. -pub fn run(command: &str, stdin_bytes: &[u8]) -> Result> { - let mut child = Command::new("sh") - .arg("-c") - .arg(command) +pub fn run(command: &mut Command, stdin_bytes: &[u8]) -> Result> { + let mut child = command .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()) .spawn() - .map_err(|err| anyhow!("spawn `{command}`: {err}"))?; + .map_err(|err| anyhow!("spawn `{command:?}`: {err}"))?; if let Some(mut stdin) = child.stdin.take() { stdin .write_all(stdin_bytes) - .map_err(|err| anyhow!("write stdin to `{command}`: {err}"))?; + .map_err(|err| anyhow!("write stdin to `{command:?}`: {err}"))?; } let output = child .wait_with_output() - .map_err(|err| anyhow!("wait `{command}`: {err}"))?; + .map_err(|err| anyhow!("wait `{command:?}`: {err}"))?; if !output.status.success() { bail!( - "`{command}` exited with status {}", + "`{command:?}` exited with status {}", output .status .code()