refactor(maestro): delete the config projections and the dead knobs
maestro-cli/src/config.rs was two pass-throughs, a newtype error, a two-field projection of MaestroConfigFile, and one four-line or_else with a single caller. `resolve_daemon_url` moves onto MaestroConfigFile, next to the schema it reads, and the module goes; its tests move with it. `[web].dev_proxy_target` is deleted end to end — schema, env var, normalisation, three tests, README line, and the startup warning that admitted the server ignores it. MaestroConfigFile::save had no callers outside its own tests. Deleting it also removes every skip_serializing_if, the five is_empty impls, and the Serialize derives that existed only to make its output tidy. BroadcastTuning was a struct that was never instantiated: all three uses were `::DEFAULT.field` in const position and there is no [broadcast] section. Now two plain consts. $MAESTRO_CONFIG was honoured twice, by resolve_config_path and by clap's `env =`. Every binary already gets it through clap, so resolve_config_path and CONFIG_PATH_ENV go and the TUI falls back to default_config_path like everything else. Also collapses three layers of 1:1 forwarding: the umbrella's cli_dispatch shim, and four near-identical 8-line functions in cli/dispatch.rs that differed only in which run_* they called. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit was merged in pull request #46.
This commit is contained in:
co-authored by
Claude Opus 5
parent
b20651c33e
commit
6596104789
@@ -1,128 +0,0 @@
|
||||
//! CLI view over the unified Maestro config at `~/.config/maestro/config.toml`.
|
||||
//!
|
||||
//! Reads `[cli].daemon_url` and, as a fallback, `[daemon].bind_addr`.
|
||||
|
||||
use std::net::SocketAddr;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use maestro_config::MaestroConfigFile;
|
||||
|
||||
pub use maestro_config::CONFIG_PATH_ENV;
|
||||
|
||||
/// Last-resort daemon HTTP base when neither config nor a local
|
||||
/// `[daemon].bind_addr` tells us otherwise. Matches the daemon's default
|
||||
/// `bind_addr`.
|
||||
pub const DEFAULT_DAEMON_URL: &str = "http://127.0.0.1:7070";
|
||||
|
||||
/// `~/.config/maestro/config.toml` (XDG-aware).
|
||||
pub fn default_config_path() -> Result<PathBuf, ConfigError> {
|
||||
Ok(maestro_config::default_config_path()?)
|
||||
}
|
||||
|
||||
/// `$MAESTRO_CONFIG` if set, else [`default_config_path`].
|
||||
pub fn resolve_config_path() -> Result<PathBuf, ConfigError> {
|
||||
Ok(maestro_config::resolve_config_path()?)
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum ConfigError {
|
||||
#[error(transparent)]
|
||||
Inner(#[from] maestro_config::ConfigError),
|
||||
}
|
||||
|
||||
/// Resolved CLI config — the subset of the unified file that this binary
|
||||
/// actually consumes.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct CliConfig {
|
||||
/// `[cli].daemon_url` — explicit daemon HTTP base.
|
||||
pub daemon_url: Option<String>,
|
||||
/// `[daemon].bind_addr` if a daemon is configured on this host. Used to
|
||||
/// derive the default daemon URL when `daemon_url` is unset.
|
||||
pub daemon_bind_addr: Option<SocketAddr>,
|
||||
}
|
||||
|
||||
impl CliConfig {
|
||||
/// Load the subset of the unified file the CLI consumes. A missing file is
|
||||
/// fine — every field has a default.
|
||||
pub fn load(path: &Path) -> Result<Self, ConfigError> {
|
||||
let file = MaestroConfigFile::load(path)?;
|
||||
Ok(Self {
|
||||
daemon_url: file.cli.daemon_url,
|
||||
daemon_bind_addr: file.daemon.bind_addr,
|
||||
})
|
||||
}
|
||||
|
||||
/// Resolve the daemon HTTP base to talk to.
|
||||
///
|
||||
/// Precedence: explicit `override_url` (CLI flag / env) → `[cli].daemon_url`
|
||||
/// → `http://<[daemon].bind_addr>` → [`DEFAULT_DAEMON_URL`]. Always returns
|
||||
/// a value; reachability is decided by whether the connection succeeds.
|
||||
pub fn resolve_daemon_url(&self, override_url: Option<String>) -> String {
|
||||
override_url
|
||||
.or_else(|| self.daemon_url.clone())
|
||||
.or_else(|| self.daemon_bind_addr.map(|a| format!("http://{a}")))
|
||||
.unwrap_or_else(|| DEFAULT_DAEMON_URL.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn write_file(path: &Path, body: &str) {
|
||||
std::fs::write(path, body).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_file_returns_empty() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let cfg = CliConfig::load(&dir.path().join("config.toml")).unwrap();
|
||||
assert_eq!(cfg, CliConfig::default());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_daemon_url_precedence() {
|
||||
// override wins over everything
|
||||
let cfg = CliConfig {
|
||||
daemon_url: Some("http://from-cli-section:7070".into()),
|
||||
daemon_bind_addr: Some("127.0.0.1:9999".parse().unwrap()),
|
||||
};
|
||||
assert_eq!(
|
||||
cfg.resolve_daemon_url(Some("http://override:1".into())),
|
||||
"http://override:1"
|
||||
);
|
||||
// [cli].daemon_url next
|
||||
assert_eq!(cfg.resolve_daemon_url(None), "http://from-cli-section:7070");
|
||||
// derive from [daemon].bind_addr when no explicit url
|
||||
let cfg = CliConfig {
|
||||
daemon_bind_addr: Some("10.0.0.7:7070".parse().unwrap()),
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(cfg.resolve_daemon_url(None), "http://10.0.0.7:7070");
|
||||
// built-in default when nothing is configured
|
||||
let cfg = CliConfig::default();
|
||||
assert_eq!(cfg.resolve_daemon_url(None), DEFAULT_DAEMON_URL);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_reads_daemon_url_and_daemon_bind_addr() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("config.toml");
|
||||
write_file(
|
||||
&path,
|
||||
r#"
|
||||
[cli]
|
||||
daemon_url = "http://laptop:7070"
|
||||
|
||||
[daemon]
|
||||
bind_addr = "127.0.0.1:7071"
|
||||
"#,
|
||||
);
|
||||
let cfg = CliConfig::load(&path).unwrap();
|
||||
assert_eq!(cfg.daemon_url.as_deref(), Some("http://laptop:7070"));
|
||||
assert_eq!(
|
||||
cfg.daemon_bind_addr,
|
||||
Some("127.0.0.1:7071".parse().unwrap())
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -9,15 +9,15 @@ use std::path::{Path, PathBuf};
|
||||
use maestro_protocol::DaemonClient;
|
||||
|
||||
use crate::attach::{parse_detach_key, run_attach_url};
|
||||
use crate::cli::{AttachArgs, Command, KillArgs, ListArgs, SpawnArgs};
|
||||
use crate::config::{default_config_path, CliConfig, ConfigError};
|
||||
use crate::cli::Command;
|
||||
use crate::sessions::{run_kill, run_list, run_spawn};
|
||||
use maestro_config::{default_config_path, MaestroConfigFile};
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
#[allow(clippy::result_large_err)]
|
||||
pub enum CliError {
|
||||
#[error("config: {0}")]
|
||||
Config(#[from] ConfigError),
|
||||
Config(#[from] maestro_config::ConfigError),
|
||||
#[error("{0}")]
|
||||
Client(#[from] maestro_protocol::ClientError),
|
||||
#[error("{0}")]
|
||||
@@ -39,76 +39,42 @@ pub async fn dispatch(
|
||||
None => default_config_path()?,
|
||||
};
|
||||
|
||||
let daemon = client(&config_path, daemon_url_override)?;
|
||||
|
||||
match command {
|
||||
Command::List(args) => list(&config_path, daemon_url_override, args).await,
|
||||
Command::Spawn(args) => spawn(&config_path, daemon_url_override, args).await,
|
||||
Command::Attach(args) => attach(&config_path, daemon_url_override, args).await,
|
||||
Command::Kill(args) => kill(&config_path, daemon_url_override, args).await,
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::result_large_err)]
|
||||
async fn list(
|
||||
config_path: &Path,
|
||||
daemon_url_override: Option<String>,
|
||||
args: ListArgs,
|
||||
) -> Result<(), CliError> {
|
||||
let daemon = client(config_path, daemon_url_override)?;
|
||||
let mut stdout = std::io::stdout().lock();
|
||||
run_list(&daemon, &args, &mut stdout).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[allow(clippy::result_large_err)]
|
||||
async fn spawn(
|
||||
config_path: &Path,
|
||||
daemon_url_override: Option<String>,
|
||||
args: SpawnArgs,
|
||||
) -> Result<(), CliError> {
|
||||
let daemon = client(config_path, daemon_url_override)?;
|
||||
let attach_after = args.attach;
|
||||
// `spawn --attach` has no detach-key flag; use the attach default (Ctrl-q).
|
||||
let detach_key = 0x11;
|
||||
|
||||
let session = run_spawn(&daemon, args).await?;
|
||||
println!("spawned {}", session.id);
|
||||
|
||||
if attach_after {
|
||||
let url = daemon.attach_url(session.id)?;
|
||||
run_attach_url(&url, detach_key).await?;
|
||||
Command::List(args) => {
|
||||
let mut stdout = std::io::stdout().lock();
|
||||
run_list(&daemon, &args, &mut stdout).await?;
|
||||
}
|
||||
Command::Spawn(args) => {
|
||||
let attach_after = args.attach;
|
||||
let session = run_spawn(&daemon, args).await?;
|
||||
println!("spawned {}", session.id);
|
||||
if attach_after {
|
||||
// `spawn --attach` has no detach-key flag; use the attach
|
||||
// default (Ctrl-q).
|
||||
run_attach_url(&daemon.attach_url(session.id)?, CTRL_Q).await?;
|
||||
}
|
||||
}
|
||||
Command::Attach(args) => {
|
||||
let detach = parse_detach_key(&args.detach_key)?;
|
||||
run_attach_url(&daemon.attach_url(args.session_id)?, detach).await?;
|
||||
}
|
||||
Command::Kill(args) => {
|
||||
run_kill(&daemon, &args).await?;
|
||||
println!("killed {}", args.session_id);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[allow(clippy::result_large_err)]
|
||||
async fn attach(
|
||||
config_path: &Path,
|
||||
daemon_url_override: Option<String>,
|
||||
args: AttachArgs,
|
||||
) -> Result<(), CliError> {
|
||||
let daemon = client(config_path, daemon_url_override)?;
|
||||
let detach = parse_detach_key(&args.detach_key)?;
|
||||
let url = daemon.attach_url(args.session_id)?;
|
||||
run_attach_url(&url, detach).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[allow(clippy::result_large_err)]
|
||||
async fn kill(
|
||||
config_path: &Path,
|
||||
daemon_url_override: Option<String>,
|
||||
args: KillArgs,
|
||||
) -> Result<(), CliError> {
|
||||
let daemon = client(config_path, daemon_url_override)?;
|
||||
run_kill(&daemon, &args).await?;
|
||||
println!("killed {}", args.session_id);
|
||||
Ok(())
|
||||
}
|
||||
/// Default detach byte, matching `maestro attach --detach-key`'s `^q`.
|
||||
const CTRL_Q: u8 = 0x11;
|
||||
|
||||
/// HTTP client pointed at the daemon's API. That surface needs no auth —
|
||||
/// reaching `bind_addr` is the authorization.
|
||||
#[allow(clippy::result_large_err)]
|
||||
fn client(config_path: &Path, override_url: Option<String>) -> Result<DaemonClient, CliError> {
|
||||
let cfg = CliConfig::load(config_path)?;
|
||||
let cfg = MaestroConfigFile::load(config_path)?;
|
||||
Ok(DaemonClient::new(cfg.resolve_daemon_url(override_url)))
|
||||
}
|
||||
|
||||
@@ -4,7 +4,6 @@
|
||||
|
||||
pub mod attach;
|
||||
pub mod cli;
|
||||
pub mod config;
|
||||
pub mod dispatch;
|
||||
pub mod sessions;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user