diff --git a/.github/workflows/ast-grep.yml b/.github/workflows/ast-grep.yml new file mode 100644 index 00000000..6d407537 --- /dev/null +++ b/.github/workflows/ast-grep.yml @@ -0,0 +1,26 @@ +# Structural rules for the command-line code (RFC-0012): lint/ast-grep's rule tests, then a scan. +# Seconds, no Rust build. +name: ast-grep + +on: + push: + branches: [main] + paths: ["lint/ast-grep/**", "apps/soundcraft-cli/**", "crates/automation/src/args.rs", "apps/soundcraft/src/launch_args.rs", "apps/soundcraft/src/main.rs", ".github/workflows/ast-grep.yml"] + pull_request: + paths: ["lint/ast-grep/**", "apps/soundcraft-cli/**", "crates/automation/src/args.rs", "apps/soundcraft/src/launch_args.rs", "apps/soundcraft/src/main.rs", ".github/workflows/ast-grep.yml"] + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ast-grep-${{ github.ref }} + cancel-in-progress: true + +jobs: + scan: + # Tracked at @main like the other shared workflows. + uses: storytold/craft-common-actions/.github/workflows/ast-grep.yml@main + with: + config: lint/ast-grep/sgconfig.yml + paths: apps/soundcraft-cli crates/automation/src/args.rs apps/soundcraft/src/launch_args.rs apps/soundcraft/src/main.rs diff --git a/ATTRIBUTION.md b/ATTRIBUTION.md index e6010d2a..00927097 100644 --- a/ATTRIBUTION.md +++ b/ATTRIBUTION.md @@ -55,3 +55,4 @@ here. The subset uses default_fonts; the parent face uses monochrome_emoji_fonts | `assets/app-icon/soundcraft.ico` | SoundCraft contributors | original, rendered from `icon.svg` | MIT OR Apache-2.0 | | `assets/app-icon/hicolor/` | SoundCraft contributors | original, rendered from `icon.svg` (16–512 px PNGs and the scalable SVG) | MIT OR Apache-2.0 | | `docs/images/` | SoundCraft contributors | screenshots of SoundCraft rendered from the synthesised demo session | MIT OR Apache-2.0 | +| `crates/audio-io/tests/fixtures/` | SoundCraft contributors | original test tones and silence generated with ffmpeg (sine) or by hand; commands in `crates/audio-io/tests/fixtures/README.md` | MIT OR Apache-2.0 | diff --git a/Cargo.lock b/Cargo.lock index 0a54619f..bbab1b86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -200,6 +200,56 @@ dependencies = [ "libc", ] +[[package]] +name = "anstream" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "824a212faf96e9acacdbd09febd34438f8f711fb84e09a8916013cd7815ca28d" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "is_terminal_polyfill", + "utf8parse", +] + +[[package]] +name = "anstyle" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" + +[[package]] +name = "anstyle-parse" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52ce7f38b242319f7cabaa6813055467063ecdc9d355bbb4ce0c68908cd8130e" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" +dependencies = [ + "windows-sys 0.61.2", +] + +[[package]] +name = "anstyle-wincon" +version = "3.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" +dependencies = [ + "anstyle", + "once_cell_polyfill", + "windows-sys 0.61.2", +] + [[package]] name = "anyhow" version = "1.0.104" @@ -738,12 +788,71 @@ dependencies = [ "libloading 0.8.9", ] +[[package]] +name = "clap" +version = "4.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa8876b300ab35ba921adea3dfd70157a46249b33f95c9084ae5709785478946" +dependencies = [ + "clap_builder", + "clap_derive", +] + [[package]] name = "clap-sys" version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a76abbdb2907f6fd97fb6bc0b7be96b77d328f2dd9669d1075cc03369ed22154" +[[package]] +name = "clap_builder" +version = "4.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0797fb7aeb1406c84efac526901f7ec3ead2124f946b494e72879d4b54704d" +dependencies = [ + "anstream", + "anstyle", + "clap_lex", + "strsim", +] + +[[package]] +name = "clap_complete" +version = "4.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "037e2a1a92236d0aff7e845093f64661d6df4c02c9fcc61a60e9e1d736fa392f" +dependencies = [ + "clap", +] + +[[package]] +name = "clap_derive" +version = "4.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f9c751b79415d4e559e3d1fcf128e09e720eb673a06d26cf6f392d37d75b66e0" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn 3.0.9", +] + +[[package]] +name = "clap_lex" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c133bc6a41be0d194c306b5506d15e6feeea7b1d6604bd3f8310dfb2ca96486" + +[[package]] +name = "clap_mangen" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "211d617eaa4b735c96c9e0228fcbdb5120ef623f2b8cb67ffb84c3e02dbc28a4" +dependencies = [ + "clap", + "roff", +] + [[package]] name = "clipboard-win" version = "5.4.3" @@ -782,6 +891,12 @@ dependencies = [ "bytemuck", ] +[[package]] +name = "colorchoice" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" + [[package]] name = "colored" version = "2.2.0" @@ -1913,6 +2028,12 @@ dependencies = [ "foldhash 0.2.0", ] +[[package]] +name = "heck" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" + [[package]] name = "hermit-abi" version = "0.5.3" @@ -2184,6 +2305,12 @@ dependencies = [ "once_cell", ] +[[package]] +name = "is_terminal_polyfill" +version = "1.70.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" + [[package]] name = "itertools" version = "0.13.0" @@ -3059,6 +3186,12 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" +[[package]] +name = "once_cell_polyfill" +version = "1.70.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" + [[package]] name = "open" version = "5.4.5" @@ -3683,6 +3816,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "roff" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "323c417e1d9665a65b263ec744ba09030cfb277e9daa0b018a4ab62e57bc8189" + [[package]] name = "rustc-hash" version = "1.1.0" @@ -4143,6 +4282,7 @@ name = "soundcraft" version = "0.13.1" dependencies = [ "chrono", + "clap", "craft-update", "eframe", "egui", @@ -4152,6 +4292,8 @@ dependencies = [ "objc2-app-kit 0.3.2", "rfd", "serde_json", + "soundcraft-audio-io", + "soundcraft-automation", "soundcraft-engine", "soundcraft-playback", "soundcraft-ui-egui", @@ -4185,10 +4327,13 @@ dependencies = [ name = "soundcraft-automation" version = "0.13.1" dependencies = [ + "clap", "log", "serde_json", + "soundcraft-audio-io", "soundcraft-engine", "soundcraft-time", + "thiserror 2.0.21", ] [[package]] @@ -4208,11 +4353,17 @@ dependencies = [ name = "soundcraft-cli" version = "0.13.1" dependencies = [ + "clap", + "clap_complete", + "clap_mangen", + "same-file", + "serde", "serde_json", "soundcraft-audio-io", "soundcraft-automation", "soundcraft-dsp", "soundcraft-engine", + "thiserror 2.0.21", ] [[package]] @@ -4380,6 +4531,7 @@ dependencies = [ "log", "rfd", "serde_json", + "soundcraft-audio-io", "soundcraft-engine", "soundcraft-model", "soundcraft-playback", @@ -4416,6 +4568,12 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fe895eb47f22e2ddd4dabc02bce419d2e643c8e3b585c78158b349195bc24d82" +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + [[package]] name = "subtle" version = "2.6.1" @@ -5043,6 +5201,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" +[[package]] +name = "utf8parse" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" + [[package]] name = "uuid" version = "1.29.0" diff --git a/Cargo.toml b/Cargo.toml index 7eecde8c..d2a9c4fc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,16 +29,21 @@ soundcraft-ui-egui = { path = "crates/ui-egui" } # foundation thiserror = "2" +clap = { version = "4", features = ["derive", "env"] } +clap_complete = "4.6.11" +clap_mangen = { version = "0.3.3", features = ["env"] } serde = { version = "1", features = ["derive", "rc"] } serde_json = { version = "1", features = ["float_roundtrip"] } log = "0.4" # All date/time conversion goes through chrono (no hand-rolled calendar code beyond the 400-year range extension; AGENTS.md). chrono = { version = "0.4.45", default-features = false, features = ["std"] } rayon = "1" +# Whether an output path is one of the inputs (the CLI refuses to replace an input unless asked). +same-file = "1" proptest = "1" # audio -symphonia = { version = "0.6", default-features = false, features = ["mp3", "flac", "vorbis", "ogg", "wav", "pcm", "aiff", "isomp4", "aac", "alac", "adpcm", "caf", "mkv"] } +symphonia = { version = "0.6", default-features = false, features = ["mp1", "mp2", "mp3", "flac", "vorbis", "ogg", "wav", "pcm", "aiff", "isomp4", "aac", "alac", "adpcm", "caf", "mkv"] } rustfft = "6" # Session bundles (one file holding a session and its audio, for the browser): stored entries only. zip = { version = "9", default-features = false } diff --git a/README.md b/README.md index 8578e95c..1024eba0 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,7 @@ error only. | Variable | Effect | |---|---| | `RUST_LOG` | Log levels for standard error and the log file. Default: `info` for SoundCraft's own crates, `warn` for everything else. env_logger-style directives replace that, e.g. `RUST_LOG=debug`, `RUST_LOG=warn,soundcraft_mix=trace` or `RUST_LOG=info,wgpu_core=warn`; a directive ending in `*` covers every target starting with it (`soundcraft*=debug`). | -| `SOUNDCRAFT_NO_PREFS` | No preferences read or written and no log file (agents' test runs). | +| `SOUNDCRAFT_NO_PREFS` | `1`: no preferences read or written and no log file (agents' test runs). | The realtime audio thread never writes a record itself: the logger keeps the first one it logs (without waiting or allocating), counts the rest, and the UI thread writes them. The logger is @@ -216,8 +216,9 @@ claude mcp add soundcraft -- soundcraft-cli mcp --demo soundcraft-cli mcp --automation-read-root DIR --automation-write-root DIR ``` -See [`docs/control-protocol.md`](docs/control-protocol.md) and [`docs/mcp.md`](docs/mcp.md). -`soundcraft-cli commands` lists every command. +See [`docs/cli.md`](docs/cli.md) for the command-line tool (subcommands, exit codes, environment +variables, man pages and completions), [`docs/control-protocol.md`](docs/control-protocol.md) and +[`docs/mcp.md`](docs/mcp.md). `soundcraft-cli commands` lists every command. Open **SoundCraft › Session Audio Health** to check loaded audio availability, sample-rate mismatches and clip source bounds across all playlists, including alternate takes. The report diff --git a/apps/soundcraft-cli/Cargo.toml b/apps/soundcraft-cli/Cargo.toml index fad22e19..9316b5b8 100644 --- a/apps/soundcraft-cli/Cargo.toml +++ b/apps/soundcraft-cli/Cargo.toml @@ -6,6 +6,10 @@ edition.workspace = true license.workspace = true publish = false +[lib] +name = "soundcraft_cli" +path = "src/lib.rs" + [[bin]] name = "soundcraft-cli" path = "src/main.rs" @@ -18,4 +22,12 @@ soundcraft-engine.workspace = true soundcraft-automation.workspace = true soundcraft-audio-io.workspace = true soundcraft-dsp.workspace = true +serde.workspace = true serde_json.workspace = true +thiserror.workspace = true +clap.workspace = true +same-file.workspace = true + +[dev-dependencies] +clap_complete.workspace = true +clap_mangen.workspace = true diff --git a/apps/soundcraft-cli/examples/man-and-completions.rs b/apps/soundcraft-cli/examples/man-and-completions.rs new file mode 100644 index 00000000..396a62ec --- /dev/null +++ b/apps/soundcraft-cli/examples/man-and-completions.rs @@ -0,0 +1,42 @@ +//! Writes soundcraft-cli's shell completions ([`clap_complete`]: bash, zsh, fish, elvish, +//! PowerShell) and man pages ([`clap_mangen`]: `soundcraft-cli.1` and one per subcommand, such as +//! `soundcraft-cli-convert.1`) into a directory. The Linux and FreeBSD packages ship them +//! (`packaging/linux/package.sh`, `packaging/freebsd/package.sh`): +//! +//! ```sh +//! cargo run -p soundcraft-cli --example man-and-completions -- +//! ``` + +use std::path::Path; +use std::process::ExitCode; + +use clap::ValueEnum; +use clap_complete::Shell; + +fn main() -> ExitCode { + let mut args = std::env::args_os().skip(1); + let (Some(dir), None) = (args.next(), args.next()) else { + eprintln!("usage: man-and-completions "); + return ExitCode::from(2); + }; + let dir = Path::new(&dir); + match generate(dir) { + Ok(()) => ExitCode::SUCCESS, + Err(e) => { + eprintln!("error: {}: {e}", dir.display()); + ExitCode::FAILURE + } + } +} + +/// Also run by `tests/it/man_and_completions.rs`. +pub(crate) fn generate(dir: &Path) -> std::io::Result<()> { + std::fs::create_dir_all(dir)?; + let mut cmd = soundcraft_cli::command(); + for &shell in Shell::value_variants() { + clap_complete::generate_to(shell, &mut cmd, "soundcraft-cli", dir)?; + } + // A fresh command: `clap_complete::generate_to` built `cmd`, adding the `help` subcommand, + // which `clap_mangen` would otherwise give pages of its own (`soundcraft-cli-help-convert.1`). + clap_mangen::generate_to(soundcraft_cli::command(), dir) +} diff --git a/apps/soundcraft-cli/src/app.rs b/apps/soundcraft-cli/src/app.rs new file mode 100644 index 00000000..32807189 --- /dev/null +++ b/apps/soundcraft-cli/src/app.rs @@ -0,0 +1,72 @@ +use super::args::AppArgs; +use super::error::CliError; +use super::output::print_json; +use serde_json::{Value, json}; +use soundcraft_automation::Remote; +use std::io::Write; + +/// Control-channel methods. Every other dotted id is an engine command. +fn is_control_method(method: &str) -> bool { + matches!( + method, + "engine.execute" + | "command" + | "engine.commands" + | "engine.parity" + | "session.inspect" + | "document.inspect" + | "ui.inspect" + | "ui.menu.list" + | "ui.menu.invoke" + | "ui.set" + | "ui.move" + | "ui.click" + | "ui.drag" + | "ui.key" + | "ui.text" + | "ui.screenshot" + | "app.quit" + ) +} + +/// A command id is shorthand for `engine.execute`. Documented control methods stay methods. +fn app_request(method: &str, params: Value) -> (String, Value) { + if is_control_method(method) || !method.contains('.') { + (method.to_string(), params) + } else { + ("engine.execute".to_string(), json!({"command": method, "params": params})) + } +} + +pub(crate) fn app(args: &AppArgs, out: &mut dyn Write) -> Result<(), CliError> { + let method = &args.method; + let params = args.json.clone().unwrap_or_else(|| json!({})); + let mut r = Remote::new(&args.control.port); + // A bare command id is shorthand for engine.execute. Control-channel methods stay methods. + let (m, p) = app_request(method, params); + let v = r.request(&m, p)?; + print_json(out, &v, args.compact); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn command_ids_are_execute_shorthand_and_control_methods_stay_methods() { + let (m, p) = app_request("engine.history", json!({})); + assert_eq!(m, "engine.execute"); + assert_eq!(p["command"], "engine.history"); + let (m, p) = app_request("ui.theme", json!({"mode": "light"})); + assert_eq!(m, "engine.execute"); + assert_eq!(p["params"]["mode"], "light"); + let (m, _) = app_request("session.new", json!({"name": "Session A"})); + assert_eq!(m, "engine.execute"); + let (m, _) = app_request("track.new", json!({})); + assert_eq!(m, "engine.execute"); + for method in ["session.inspect", "engine.commands", "engine.parity", "ui.screenshot", "app.quit"] { + assert_eq!(app_request(method, json!({})).0, method); + } + } +} diff --git a/apps/soundcraft-cli/src/args.rs b/apps/soundcraft-cli/src/args.rs new file mode 100644 index 00000000..40351779 --- /dev/null +++ b/apps/soundcraft-cli/src/args.rs @@ -0,0 +1,459 @@ +//! The command line, as clap types. A flag a subcommand doesn't take is a usage error (exit 2), so +//! a typo such as `--bounces` can't be silently ignored (#549), and so is a value flag without its +//! value: a following flag is never taken as the value. + +use std::borrow::Cow; +use std::ffi::OsString; +use std::num::NonZeroU32; +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +use clap::builder::NonEmptyStringValueParser; +use clap::error::ErrorKind; +use clap::{ArgAction, Args, CommandFactory, FromArgMatches, Parser, Subcommand, ValueEnum}; +use serde_json::{Value, json}; +use soundcraft_audio_io::FileFormat; +use soundcraft_automation::ControlTarget; +use soundcraft_automation::args::{AutomationArgs, ControlClientArgs, blank_env_is_unset, control_target, long_version, name_env_vars}; + +/// Help text that names the extensions `list` gives, built once from +/// [`FileFormat::readable_extensions`] or [`FileFormat::writable_extensions`] so it can't drift +/// from what the code accepts. +fn with_formats(cell: &'static OnceLock, before: &str, list: fn() -> Vec<&'static str>, after: &str) -> &'static str { + cell.get_or_init(|| format!("{before}({}){after}", list().join(", "))) +} + +fn info_file_help() -> &'static str { + static C: OnceLock = OnceLock::new(); + with_formats(&C, "An audio file ", FileFormat::readable_extensions, " or a .scraft session") +} + +fn read_help() -> &'static str { + static C: OnceLock = OnceLock::new(); + with_formats(&C, "The audio file to read ", FileFormat::readable_extensions, "") +} + +fn write_help() -> &'static str { + static C: OnceLock = OnceLock::new(); + with_formats(&C, "The audio file to write; its extension picks the format ", FileFormat::writable_extensions, "") +} + +/// soundcraft-cli: SoundCraft without a window +#[derive(Parser)] +#[command(name = "soundcraft-cli", version = long_version(), arg_required_else_help = true)] +pub(crate) struct Cli { + #[command(subcommand)] + pub(crate) command: Command, +} + +#[derive(Subcommand)] +pub(crate) enum Command { + Info(InfoArgs), + Convert(ConvertArgs), + Run(RunArgs), + Script(ScriptArgs), + Commands(CommandsArgs), + Describe(DescribeArgs), + App(AppArgs), + Mcp(McpArgs), + Parity(ParityArgs), + /// List the built-in plugins + Plugins, +} + +/// Print an audio file's format as JSON, or a .scraft session's summary +#[derive(Args)] +pub(crate) struct InfoArgs { + #[arg(help = info_file_help())] + pub(crate) file: PathBuf, + /// One line of JSON instead of pretty-printed (a .scraft summary is text and ignores it) + #[arg(long)] + pub(crate) compact: bool, +} + +/// Convert an audio file: resample, change the bit depth, normalize +/// +/// Dithers only when the output loses resolution, unless --dither or --no-dither says otherwise. +#[derive(Args)] +pub(crate) struct ConvertArgs { + #[arg(value_name = "IN", help = read_help())] + pub(crate) input: PathBuf, + #[arg(value_name = "OUT", help = write_help())] + pub(crate) output: PathBuf, + /// The output's bit depth + #[arg(long, value_enum, value_name = "16|24|32f", default_value = "24")] + pub(crate) bit_depth: BitDepthArg, + /// Resample to this sample rate + #[arg(long, value_name = "HZ")] + pub(crate) rate: Option, + /// Normalize the peak to -0.1 dBFS + #[arg(long)] + pub(crate) normalize: bool, + /// Always dither + #[arg(long, conflicts_with = "no_dither")] + pub(crate) dither: bool, + /// Never dither + #[arg(long)] + pub(crate) no_dither: bool, + /// Allow an that is the file, replacing the original + #[arg(long)] + pub(crate) in_place: bool, +} + +/// Where `run` and `script` start, and what they do with the result. +#[derive(Args)] +pub(crate) struct EngineArgs { + /// Open this session first + #[arg(long = "in", value_name = "S.scraft", conflicts_with = "demo")] + pub(crate) input: Option, + /// Start from the demo session + #[arg(long, visible_alias = "sample")] + pub(crate) demo: bool, + /// Bounce the mix to this file at the end; without --start and --end, the whole session + #[arg(long, value_name = "OUT")] + pub(crate) bounce: Option, + /// Where the bounce starts: seconds (12.5) or a time string + #[arg(long, value_name = "T", allow_negative_numbers = true, requires = "bounce", value_parser = time_arg)] + pub(crate) start: Option, + /// Where the bounce ends: seconds (12.5) or a time string + #[arg(long, value_name = "T", allow_negative_numbers = true, requires = "bounce", value_parser = time_arg)] + pub(crate) end: Option, + /// Save the session to this file at the end, replacing it + #[arg(long, value_name = "S.scraft")] + pub(crate) save: Option, + /// Print the session as JSON at the end + #[arg(long)] + pub(crate) inspect: bool, + /// Print the --inspect JSON on one line instead of pretty-printed + #[arg(long, requires = "inspect")] + pub(crate) compact: bool, + /// Allow a --save that is the --in session, replacing the original + #[arg(long, requires = "input")] + pub(crate) in_place: bool, +} + +/// Run engine commands on a new, demo or saved session +/// +/// Prints each command's JSON result on one line, then bounces, saves and inspects, in that order. +/// The --inspect JSON is pretty-printed unless --compact is given. +#[derive(Args)] +pub(crate) struct RunArgs { + /// An engine command, with its JSON parameters after `=` (default {}); repeat for several, run + /// in order + #[arg(long, value_name = "ID[=JSON]", action = ArgAction::Append, value_parser = cmd_spec)] + pub(crate) cmd: Vec, + #[command(flatten)] + pub(crate) engine: EngineArgs, +} + +/// Run a file of engine commands, one `ID JSON` per line +/// +/// Blank lines and lines starting with # are skipped. A failing line stops the script unless +/// --keep-going is given. +#[derive(Args)] +pub(crate) struct ScriptArgs { + /// The script to run + pub(crate) file: PathBuf, + /// Report a failing line and go on with the next + #[arg(long)] + pub(crate) keep_going: bool, + #[command(flatten)] + pub(crate) engine: EngineArgs, +} + +/// List the engine commands +#[derive(Args)] +pub(crate) struct CommandsArgs { + /// Only the commands whose id, or label in any case, contains this text + #[arg(value_parser = NonEmptyStringValueParser::new())] + pub(crate) filter: Option, +} + +/// Print one engine command's details as JSON +#[derive(Args)] +pub(crate) struct DescribeArgs { + /// The command's id, such as file.bounce_mix + #[arg(value_parser = NonEmptyStringValueParser::new())] + pub(crate) id: String, + /// One line of JSON instead of pretty-printed + #[arg(long)] + pub(crate) compact: bool, +} + +/// Call a running app's control channel +/// +/// A dotted engine command id is shorthand for engine.execute; the control methods +/// (session.inspect, ui.screenshot, …) stay methods. Prints the reply as JSON. +#[derive(Args)] +pub(crate) struct AppArgs { + #[command(flatten)] + pub(crate) control: ControlClientArgs, + /// A control method or engine command id + #[arg(value_parser = NonEmptyStringValueParser::new())] + pub(crate) method: String, + /// Its parameters as JSON [default: {}] + #[arg(value_parser = json_value)] + pub(crate) json: Option, + /// One line of JSON instead of pretty-printed + #[arg(long)] + pub(crate) compact: bool, +} + +/// Serve MCP on stdin and stdout +/// +/// The automation roots confine every file the agent's commands read (open, import, presets) and +/// write (save, bounce, export) to those folders, links followed. Each root flag may be given more +/// than once, a path inside any of its folders being allowed, and a kind left out grants none of +/// its access. The roots confine the in-process server, so they, like --demo, conflict with --connect. +#[derive(Args)] +pub(crate) struct McpArgs { + /// Serve a running app's control channel at this port or host:port instead of an in-process + /// engine; conflicts with --demo, which starts only that engine + #[arg(long, value_name = "PORT", value_parser = control_target, conflicts_with_all = ["demo", "automation_read_root", "automation_write_root"])] + pub(crate) connect: Option, + /// Start the in-process engine from the demo session + #[arg(long)] + pub(crate) demo: bool, + #[command(flatten)] + pub(crate) automation: AutomationArgs, +} + +/// Print the parity report (markdown) +#[derive(Args)] +pub(crate) struct ParityArgs { + /// Write the report to this file instead + #[arg(long, value_name = "PATH")] + pub(crate) write: Option, +} + +impl Command { + /// The outputs clap can't check: one that is an input file, however it is spelt, or both of + /// `--bounce` and `--save` naming one file. The paths are compared before any work starts and + /// written later, so a link swapped in meanwhile goes unnoticed: this guards against mistakes, + /// it is not a security boundary. + fn check_outputs(&self) -> Result<(), OutputError> { + match self { + Self::Convert(a) if !a.in_place && same_file(&a.input, &a.output) => { + Err(OutputError::ReplacesInput { output: "", input: " file", path: a.output.clone() }) + } + Self::Run(a) => a.engine.check_outputs(None), + Self::Script(a) => a.engine.check_outputs(Some(&a.file)), + Self::Info(_) + | Self::Convert(_) + | Self::Commands(_) + | Self::Describe(_) + | Self::App(_) + | Self::Mcp(_) + | Self::Parity(_) + | Self::Plugins => Ok(()), + } + } +} + +impl ConvertArgs { + /// The file `convert` writes: , or the file it links to when it replaces in place. + pub(crate) fn write_path(&self) -> Cow<'_, Path> { + in_place_target(self.in_place, Some(&self.input), &self.output) + } +} + +impl EngineArgs { + /// The file --save writes: its path, or the file it links to when it replaces --in in place. + pub(crate) fn save_path(&self) -> Option> { + self.save.as_deref().map(|save| in_place_target(self.in_place, self.input.as_deref(), save)) + } + + /// `script` is the script file `script` runs, which no flag lets an output replace. --in-place + /// lets only --save replace the --in session: a bounce would turn it into audio. + fn check_outputs(&self, script: Option<&Path>) -> Result<(), OutputError> { + for (flag, path) in [("--bounce", &self.bounce), ("--save", &self.save)] { + let Some(path) = path else { continue }; + if script.is_some_and(|script| same_file(script, path)) { + return Err(OutputError::ReplacesScript { output: flag, path: path.clone() }); + } + } + let replaces_input = |path: &Path| self.input.as_ref().is_some_and(|input| same_file(input, path)); + if let Some(bounce) = &self.bounce + && replaces_input(bounce) + { + return Err(OutputError::BounceReplacesInput(bounce.clone())); + } + if let Some(save) = &self.save + && !self.in_place + && replaces_input(save) + { + return Err(OutputError::ReplacesInput { output: "--save", input: "--in session", path: save.clone() }); + } + match (&self.bounce, &self.save) { + (Some(bounce), Some(save)) if same_file(bounce, save) => Err(OutputError::SameOutputs(save.clone())), + _ => Ok(()), + } + } +} + +/// Whether `a` and `b` name one file, however spelt (`a.wav`, `./a.wav`, a symbolic or hard link). +/// When either exists, only two regular files can be one, compared by identity (device and inode); +/// a FIFO or device is never opened, so the check can't block. When neither exists yet, they are +/// one if they would be created at the same place. +fn same_file(a: &Path, b: &Path) -> bool { + if a.exists() || b.exists() { + return a.is_file() && b.is_file() && same_file::is_same_file(a, b).unwrap_or(false); + } + matches!((destination(a), destination(b)), (Some(a), Some(b)) if a == b) +} + +/// With `in_place`, an `output` that is a symbolic link to `input` resolved to the file it names. +/// A save writes a new file and renames it over its path, which would replace the link and leave +/// the input as it was: through the resolved path, `convert` and `--save` both replace the input. +fn in_place_target<'a>(in_place: bool, input: Option<&Path>, output: &'a Path) -> Cow<'a, Path> { + let link = output.symlink_metadata().is_ok_and(|m| m.file_type().is_symlink()); + if in_place + && link + && input.is_some_and(|input| same_file(input, output)) + && let Ok(target) = output.canonicalize() + { + return Cow::Owned(target); + } + Cow::Borrowed(output) +} + +/// Where a file that doesn't exist yet would be created: its folder resolved, links followed. +fn destination(path: &Path) -> Option { + let folder = path.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or(Path::new(".")); + Some(folder.canonicalize().ok()?.join(path.file_name()?)) +} + +/// An output [`Command::check_outputs`] refuses: `parse` prints it as a usage error (exit 2). +#[derive(Debug, thiserror::Error)] +pub(crate) enum OutputError { + #[error("{output} {} is the {input}, which it would replace: choose another file, or pass --in-place", .path.display())] + ReplacesInput { output: &'static str, input: &'static str, path: PathBuf }, + #[error("--bounce {} is the --in session, which it would replace with audio: choose another file", .0.display())] + BounceReplacesInput(PathBuf), + #[error("{output} {} is the script file, which it would replace: choose another file", .path.display())] + ReplacesScript { output: &'static str, path: PathBuf }, + #[error("--bounce and --save both name {}, so the save would replace the bounce: choose another file for one of them", .0.display())] + SameOutputs(PathBuf), +} + +impl OutputError { + /// Two arguments that may not name one file: clap's conflict kind. + fn into_clap(self, cmd: &mut clap::Command) -> clap::Error { + cmd.error(ErrorKind::ArgumentConflict, self) + } +} + +/// The bit depths `convert` writes. `32` is the same as `32f`. +#[derive(Clone, Copy, ValueEnum)] +pub(crate) enum BitDepthArg { + #[value(name = "16")] + Int16, + #[value(name = "24")] + Int24, + #[value(name = "32f", alias = "32")] + Float32, +} + +impl From for soundcraft_audio_io::BitDepth { + fn from(depth: BitDepthArg) -> Self { + match depth { + BitDepthArg::Int16 => Self::Int16, + BitDepthArg::Int24 => Self::Int24, + BitDepthArg::Float32 => Self::Float32, + } + } +} + +/// A bounce position: seconds, or a time string the engine reads. +#[derive(Clone, Debug, PartialEq)] +pub(crate) enum TimeArg { + Seconds(f64), + Text(String), +} + +impl TimeArg { + /// The `start` / `end` parameter of `file.bounce_mix`. + pub(crate) fn to_json(&self) -> Value { + match self { + Self::Seconds(s) => json!({"seconds": s}), + Self::Text(t) => json!(t), + } + } +} + +/// One `--cmd ID[=JSON]`: the command id and its parameters (`{}` when none are given). +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct CmdSpec { + pub(crate) id: String, + pub(crate) params: Value, +} + +/// Why a value was refused: clap prints it after the flag and its value (exit 2). +#[derive(Debug, thiserror::Error)] +pub(crate) enum ValueError { + #[error("expected seconds (12.5) or a time string, not an empty value")] + EmptyTime, + #[error("expected a finite number of seconds")] + NotFinite, + #[error("expected a command id, optionally followed by `=` and JSON parameters")] + EmptyCommandId, + #[error("bad JSON: {0}")] + Json(#[from] serde_json::Error), +} + +fn time_arg(s: &str) -> Result { + if s.is_empty() { + return Err(ValueError::EmptyTime); + } + match s.parse::() { + // JSON has no NaN or infinity: serde_json would send the engine `null`. + Ok(secs) if !secs.is_finite() => Err(ValueError::NotFinite), + Ok(secs) => Ok(TimeArg::Seconds(secs)), + Err(_) => Ok(TimeArg::Text(s.to_owned())), + } +} + +fn json_value(s: &str) -> Result { + Ok(serde_json::from_str(s)?) +} + +/// `ID` or `ID=JSON`; malformed JSON is an error, not `{}`. +fn cmd_spec(s: &str) -> Result { + let (id, params) = match s.split_once('=') { + Some((id, p)) => (id, json_value(p)?), + None => (s, json!({})), + }; + if id.is_empty() { + return Err(ValueError::EmptyCommandId); + } + Ok(CmdSpec { id: id.to_owned(), params }) +} + +/// Parse the arguments (without the binary name). clap reads them as `OsString`s: a folder need +/// not be valid Unicode, text must. A flag wins over its environment variable, unless that is +/// empty, and a bad value from one is a usage error naming the variable. A bare `--help` or `-h` +/// is help even after a mistake on the same line (`run --bounce --help`): clap reports the mistake +/// first. An output that would replace an input is refused here too +/// ([`Command::check_outputs`]), so nothing has run when it is reported. +pub(crate) fn parse(args: &[OsString]) -> Result { + let mut cmd = Cli::command().mut_subcommands(|sub| sub.mut_args(blank_env_is_unset)); + let argv = std::iter::once(OsString::from("soundcraft-cli")).chain(args.iter().cloned()); + let matches = match cmd.try_get_matches_from_mut(argv) { + Err(e) if e.kind() != ErrorKind::DisplayHelp => { + let Some(flag) = args.iter().take_while(|a| *a != "--").find(|a| *a == "--help" || *a == "-h") else { + return Err(name_env_vars(e, &cmd, args)); + }; + let help = [OsString::from("soundcraft-cli"), args.first().cloned().unwrap_or_default(), flag.clone()]; + return Err(cmd.try_get_matches_from_mut(help).err().unwrap_or(e)); + } + matches => matches?, + }; + let cli = Cli::from_arg_matches(&matches)?; + if let Err(e) = cli.command.check_outputs() { + return Err(match matches.subcommand_name().and_then(|name| cmd.find_subcommand_mut(name)) { + Some(sub) => e.into_clap(sub), + None => e.into_clap(&mut cmd), + }); + } + Ok(cli) +} diff --git a/apps/soundcraft-cli/src/automation_args.rs b/apps/soundcraft-cli/src/automation_args.rs deleted file mode 100644 index e8135f91..00000000 --- a/apps/soundcraft-cli/src/automation_args.rs +++ /dev/null @@ -1,84 +0,0 @@ -//! Parse `--automation-read-root` / `--automation-write-root` before the server starts. A malformed -//! flag must not silently remove a root and leave the server unconfined. - -use std::path::PathBuf; - -#[derive(Default)] -pub struct AutomationArgs { - pub read_roots: Vec, - pub write_roots: Vec, -} - -impl AutomationArgs { - /// Returns whether this argument is a root flag. Each flag may be given more than once (#874), - /// in either spelling (`--flag DIR`, `--flag=DIR`): every use adds a folder. - pub fn consume(&mut self, arg: &str, args: &mut impl Iterator) -> Result { - let (flag, inline) = arg.split_once('=').map_or((arg, None), |(flag, value)| (flag, Some(value))); - let roots = match flag { - "--automation-read-root" => &mut self.read_roots, - "--automation-write-root" => &mut self.write_roots, - _ => return Ok(false), - }; - let value = match inline { - Some(value) => value.to_owned(), - None => { - let value = args.next().ok_or_else(|| format!("{flag} needs a folder"))?; - if value.starts_with('-') { - return Err(format!("{flag} needs a folder; use {flag}= for a folder starting with '-'")); - } - value - } - }; - if value.is_empty() { - return Err(format!("{flag} needs a non-empty folder")); - } - roots.push(value.into()); - Ok(true) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn roots_add_up_and_other_arguments_are_left() { - let mut roots = AutomationArgs::default(); - let mut args = ["a folder".to_owned(), "song.scraft".to_owned()].into_iter(); - assert!(roots.consume("--automation-read-root", &mut args).unwrap()); - assert!(roots.consume("--automation-write-root=out", &mut args).unwrap()); - assert!(!roots.consume("--demo", &mut args).unwrap()); - assert_eq!(roots.read_roots, [PathBuf::from("a folder")]); - assert_eq!(roots.write_roots, [PathBuf::from("out")]); - assert_eq!(args.next().as_deref(), Some("song.scraft")); - // #874: a flag given again adds a folder, in either spelling. - assert!(roots.consume("--automation-read-root=other", &mut args).unwrap()); - assert!(roots.consume("--automation-write-root", &mut ["out2".to_owned()].into_iter()).unwrap()); - assert_eq!(roots.read_roots, [PathBuf::from("a folder"), PathBuf::from("other")]); - assert_eq!(roots.write_roots, [PathBuf::from("out"), PathBuf::from("out2")]); - } - - #[test] - fn invalid_operands_leave_roots_unchanged() { - for flag in ["--automation-read-root", "--automation-write-root"] { - for operand in [None, Some(""), Some("--version"), Some("--connect"), Some("-folder")] { - let mut roots = AutomationArgs { read_roots: vec!["read".into()], write_roots: vec!["write".into()] }; - let mut args = operand.map(str::to_owned).into_iter(); - assert!(roots.consume(flag, &mut args).is_err(), "{flag} {operand:?}"); - assert_eq!(roots.read_roots, [PathBuf::from("read")]); - assert_eq!(roots.write_roots, [PathBuf::from("write")]); - } - let mut roots = AutomationArgs::default(); - assert!(roots.consume(&format!("{flag}="), &mut std::iter::empty()).is_err()); - } - } - - #[test] - fn explicit_dash_folder_and_relative_dash_folder_are_valid() { - let mut roots = AutomationArgs::default(); - assert!(roots.consume("--automation-read-root=-folder", &mut std::iter::empty()).unwrap()); - assert!(roots.consume("--automation-write-root", &mut ["./-folder".to_owned()].into_iter()).unwrap()); - assert_eq!(roots.read_roots, [PathBuf::from("-folder")]); - assert_eq!(roots.write_roots, [PathBuf::from("./-folder")]); - } -} diff --git a/apps/soundcraft-cli/src/catalog.rs b/apps/soundcraft-cli/src/catalog.rs new file mode 100644 index 00000000..c701d66c --- /dev/null +++ b/apps/soundcraft-cli/src/catalog.rs @@ -0,0 +1,50 @@ +use super::args::{CommandsArgs, DescribeArgs, ParityArgs}; +use super::error::CliError; +use super::output::print_json; +use soundcraft_engine::Engine; +use std::io::Write; + +pub(crate) fn commands(args: &CommandsArgs, out: &mut dyn Write) { + let filter = args.filter.clone().unwrap_or_default(); + let e = Engine::default(); + for c in soundcraft_engine::command_specs() { + if !filter.is_empty() && !c.id.contains(&filter) && !c.label.to_ascii_lowercase().contains(&filter.to_ascii_lowercase()) { + continue; + } + let i = c.info(&e); + outln!(out, "{:<36} {:<34} {}", i.id, i.label, i.menu.join(" > ")); + } +} + +pub(crate) fn describe(args: &DescribeArgs, out: &mut dyn Write) -> Result<(), CliError> { + let id = &args.id; + let c = soundcraft_engine::find_command(id).ok_or_else(|| CliError::UnknownCommand(id.clone()))?; + print_json(out, &c.info(&Engine::default()), args.compact); + Ok(()) +} + +pub(crate) fn parity(args: &ParityArgs, out: &mut dyn Write) -> Result<(), CliError> { + let p = soundcraft_engine::catalog::parity_json(); + let mut md = String::new(); + md.push_str("# Parity report\n\nGenerated by `soundcraft-cli parity --write docs/parity-checklist.md` (also `cargo xtask parity`).\n"); + md.push_str("It compares the incumbent's menu tree (feature names observed black-box, `crates/engine/catalog/menus.txt`) with SoundCraft's command registry.\n"); + md.push_str("UI-layer commands (windows, AudioSuite) are counted by the app's own report (`engine.parity` over the control channel).\n\n"); + md.push_str(&format!( + "**Menu items implemented by the engine: {} / {} ({}%)**\n\n| Menu | Implemented | Total |\n|---|---:|---:|\n", + p["implemented"], p["total"], p["percent"] + )); + for m in p["per_menu"].as_array().cloned().unwrap_or_default() { + md.push_str(&format!("| {} | {} | {} |\n", m["menu"].as_str().unwrap_or(""), m["implemented"], m["total"])); + } + md.push_str("\n## Not yet implemented\n\n"); + for m in p["missing"].as_array().cloned().unwrap_or_default() { + md.push_str(&format!("- {}\n", m.as_str().unwrap_or(""))); + } + if let Some(path) = &args.write { + std::fs::write(path, &md).map_err(|source| CliError::File { path: path.clone(), source })?; + outln!(out, "wrote {}: {}% ({}/{})", path.display(), p["percent"], p["implemented"], p["total"]); + } else { + outln!(out, "{md}"); + } + Ok(()) +} diff --git a/apps/soundcraft-cli/src/convert.rs b/apps/soundcraft-cli/src/convert.rs new file mode 100644 index 00000000..91c606b8 --- /dev/null +++ b/apps/soundcraft-cli/src/convert.rs @@ -0,0 +1,105 @@ +use super::args::ConvertArgs; +use super::error::{CliError, ConvertError}; +use std::io::Write; +use std::num::NonZeroU32; + +pub(crate) fn convert(args: &ConvertArgs, out: &mut dyn Write) -> Result<(), CliError> { + let (input, output) = (&args.input, &args.output); + let rate = args.rate.map(NonZeroU32::get); + let bit_depth = args.bit_depth.into(); + let dither_flag = match (args.dither, args.no_dither) { + (true, _) => Some(true), + (_, true) => Some(false), + _ => None, + }; + let bytes = std::fs::read(input).map_err(|source| CliError::File { path: input.clone(), source })?; + let ext = input.extension().and_then(std::ffi::OsStr::to_str); + let (info, mut buf) = soundcraft_audio_io::decode(&bytes, ext).map_err(|source| CliError::Audio { path: input.clone(), source })?; + if let Some(rate) = rate { + buf.channels = soundcraft_dsp::offline::resample(&buf.channels, buf.sample_rate, rate); + buf.sample_rate = rate; + } + let normalize = args.normalize; + if normalize { + soundcraft_dsp::offline::normalize(&mut buf.channels, -0.1, false); + } + let processed = rate.is_some_and(|r| r != info.sample_rate) || normalize; + let dither = dither_flag.unwrap_or_else(|| auto_dither(info.sample_format, bit_depth, processed)); + // Lossy only for the message: an extension that is not valid Unicode names no format. + let out_ext = output.extension().map_or_else(|| "wav".into(), |e| e.to_string_lossy().to_ascii_lowercase()); + let Some(format) = soundcraft_audio_io::FileFormat::from_write_extension(&out_ext) else { + return Err(ConvertError::OutputFormat { output: output.clone(), ext: out_ext }.into()); + }; + let opts = soundcraft_audio_io::EncodeOptions { format, bit_depth, dither, bwf: None }; + let bytes = soundcraft_audio_io::encode(&buf, &opts).map_err(|source| CliError::Audio { path: output.clone(), source })?; + write_replacing(&args.write_path(), &bytes).map_err(|source| CliError::File { path: output.clone(), source })?; + outln!(out, "wrote {} ({} ch, {} Hz, {:.2} s)", output.display(), buf.num_channels(), buf.sample_rate, buf.duration_secs()); + Ok(()) +} + +/// Writes `bytes` to a new file next to `target`, flushes it and renames it over `target`, so `target` +/// (the input, for --in-place) holds either its old or its new contents. The new file is removed when +/// writing or renaming fails. +fn write_replacing(target: &std::path::Path, bytes: &[u8]) -> std::io::Result<()> { + let name = target.file_name().map_or_else(|| "output".into(), |n| n.to_string_lossy().into_owned()); + let tmp = target.with_file_name(format!(".{name}.{}.part", std::process::id())); + let written = (|| { + let mut file = std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp)?; + file.write_all(bytes)?; + file.sync_all()?; + std::fs::rename(&tmp, target) + })(); + if written.is_err() { + let _ = std::fs::remove_file(&tmp); + } + written +} + +/// Whether `convert` dithers when not told: only when writing integer samples that cannot hold the +/// source exactly, i.e. the output has fewer bits than the source, the source is float or lossy, or +/// resampling/normalizing produced new sample values. A 16-bit file written as 16 or 24-bit, or +/// anything written as 32-bit float, is copied sample for sample (digital silence stays silent). +fn auto_dither(source: soundcraft_audio_io::SampleFormat, target: soundcraft_audio_io::BitDepth, processed: bool) -> bool { + use soundcraft_audio_io::{BitDepth, SampleFormat}; + let target_bits = match target { + BitDepth::Float32 => return false, + BitDepth::Int16 => 16, + BitDepth::Int24 => 24, + BitDepth::Int32 => 32, + }; + if processed { + return true; + } + match source { + SampleFormat::Int8 => target_bits < 8, + SampleFormat::Int16 => target_bits < 16, + SampleFormat::Int24 => target_bits < 24, + // Decoded through f32, which holds 24 bits: a 32-bit integer source has already lost + // resolution, so treat it like float. + SampleFormat::Int32 | SampleFormat::Float32 | SampleFormat::Float64 | SampleFormat::Compressed => true, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn convert_dithers_only_when_resolution_is_lost() { + use soundcraft_audio_io::{BitDepth, SampleFormat}; + // Same depth or wider: the samples fit exactly. + assert!(!auto_dither(SampleFormat::Int16, BitDepth::Int16, false)); + assert!(!auto_dither(SampleFormat::Int16, BitDepth::Int24, false)); + assert!(!auto_dither(SampleFormat::Int24, BitDepth::Int24, false)); + assert!(!auto_dither(SampleFormat::Int8, BitDepth::Int16, false)); + // Float output never dithers. + assert!(!auto_dither(SampleFormat::Float32, BitDepth::Float32, true)); + assert!(!auto_dither(SampleFormat::Int24, BitDepth::Float32, false)); + // Fewer bits, a float or lossy source, or processed samples: dither. + assert!(auto_dither(SampleFormat::Int24, BitDepth::Int16, false)); + assert!(auto_dither(SampleFormat::Float32, BitDepth::Int24, false)); + assert!(auto_dither(SampleFormat::Compressed, BitDepth::Int16, false)); + assert!(auto_dither(SampleFormat::Int32, BitDepth::Int24, false)); + assert!(auto_dither(SampleFormat::Int16, BitDepth::Int16, true)); + } +} diff --git a/apps/soundcraft-cli/src/error.rs b/apps/soundcraft-cli/src/error.rs new file mode 100644 index 00000000..a048694c --- /dev/null +++ b/apps/soundcraft-cli/src/error.rs @@ -0,0 +1,49 @@ +use std::path::{Path, PathBuf}; + +use soundcraft_audio_io::AudioError; +use soundcraft_automation::RemoteError; +use soundcraft_engine::EngineError; +use soundcraft_engine::file_access::RootError; + +/// Why a subcommand failed: printed as `error: {e}`, exit 1. +#[derive(Debug, thiserror::Error)] +pub(crate) enum CliError { + #[error(transparent)] + Convert(#[from] ConvertError), + #[error(transparent)] + Engine(#[from] EngineError), + #[error("unknown command `{0}`")] + UnknownCommand(String), + #[error("{id}: {source}")] + Command { id: String, source: EngineError }, + #[error("{}:{line}: bad JSON: {source}", path.display())] + ScriptJson { path: PathBuf, line: usize, source: serde_json::Error }, + #[error("{}:{line}: {id}: {source}", path.display())] + ScriptLine { path: PathBuf, line: usize, id: String, source: EngineError }, + #[error("{}: {source}", path.display())] + File { path: PathBuf, source: std::io::Error }, + #[error("{}: {source}", path.display())] + Audio { path: PathBuf, source: AudioError }, + /// A `--bounce` or `--save` path: the engine's commands take them in JSON, which holds only + /// text. + #[error("{}: the path is not valid Unicode", .0.display())] + NotUnicode(PathBuf), + #[error(transparent)] + Remote(#[from] RemoteError), + #[error(transparent)] + Roots(#[from] RootError), + #[error(transparent)] + Serve(std::io::Error), +} + +/// A path as the text the engine's commands take. +pub(crate) fn unicode(path: &Path) -> Result<&str, CliError> { + path.to_str().ok_or_else(|| CliError::NotUnicode(path.to_owned())) +} + +/// A `convert` option or output the tool refuses. +#[derive(Debug, PartialEq, thiserror::Error)] +pub(crate) enum ConvertError { + #[error("{}: cannot write `.{ext}` files (supported: {})", output.display(), soundcraft_audio_io::FileFormat::writable_extensions().join(", "))] + OutputFormat { output: PathBuf, ext: String }, +} diff --git a/apps/soundcraft-cli/src/info.rs b/apps/soundcraft-cli/src/info.rs new file mode 100644 index 00000000..51f9264c --- /dev/null +++ b/apps/soundcraft-cli/src/info.rs @@ -0,0 +1,27 @@ +use super::args::InfoArgs; +use super::error::CliError; +use super::output::print_json; +use super::session::warn_missing; +use serde_json::json; +use soundcraft_engine::Engine; +use std::io::Write; + +pub(crate) fn info(args: &InfoArgs, out: &mut dyn Write, err: &mut dyn Write) -> Result<(), CliError> { + let path = &args.file; + if path.extension().is_some_and(|ext| ext.eq_ignore_ascii_case("scraft")) { + let mut e = Engine::default(); + let missing = soundcraft_engine::io::open_session(&mut e, path)?; + warn_missing(err, &missing); + outln!(out, "{}", soundcraft_engine::inspect::session_text(&e)); + return Ok(()); + } + let bytes = std::fs::read(path).map_err(|source| CliError::File { path: path.clone(), source })?; + let ext = path.extension().and_then(std::ffi::OsStr::to_str); + let i = soundcraft_audio_io::probe(&bytes, ext).map_err(|source| CliError::Audio { path: path.clone(), source })?; + print_json( + out, + &json!({"format": i.format, "sample_format": i.sample_format, "sample_rate": i.sample_rate, "channels": i.channels, "frames": i.frames, "seconds": i.duration_secs(), "bwf": i.bwf}), + args.compact, + ); + Ok(()) +} diff --git a/apps/soundcraft-cli/src/lib.rs b/apps/soundcraft-cli/src/lib.rs new file mode 100644 index 00000000..fe351e2e --- /dev/null +++ b/apps/soundcraft-cli/src/lib.rs @@ -0,0 +1,82 @@ +//! `soundcraft-cli`: SoundCraft without a window. `run` is the whole program, so it can be tested +//! in-process as well as through the binary. `soundcraft-cli --help` lists the subcommands. +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] + +#[macro_use] +mod output; + +mod app; +mod args; +mod catalog; +mod convert; +mod error; +mod info; +mod mcp; +mod session; + +use std::ffi::OsString; +use std::io::Write; + +use clap::CommandFactory; + +use app::app; +use args::{Cli, Command, parse}; +use catalog::{commands, describe, parity}; +use convert::convert; +use error::CliError; +use info::info; +use mcp::mcp; +use session::{run_cmds, script}; + +/// The command line's clap definition. +pub fn command() -> clap::Command { + Cli::command() +} + +/// Run the CLI on its arguments (without the program name), writing results to `out` and +/// diagnostics to `err`. Returns the process exit code: 0 on success, 1 on failure, 2 for a usage +/// error. Help and the version go to `out`, exit 0. Plugin hosts are shut down by the caller once +/// the process is done with every engine (see `main`). +pub fn run(args: I, out: &mut dyn Write, err: &mut dyn Write) -> i32 +where + I: IntoIterator, + T: Into, +{ + let args: Vec = args.into_iter().map(Into::into).collect(); + // Help and the version (`help`, `-h`, ` --help`, `-V`) come back as errors too. + let cli = match parse(&args) { + Ok(cli) => cli, + Err(e) => { + let to: &mut dyn Write = if e.use_stderr() { err } else { out }; + let _ = write!(to, "{}", e.render()); + return e.exit_code(); + } + }; + match dispatch(cli.command, out, err) { + Ok(()) => 0, + Err(e) => { + outln!(err, "error: {e}"); + 1 + } + } +} + +fn dispatch(command: Command, out: &mut dyn Write, err: &mut dyn Write) -> Result<(), CliError> { + match command { + Command::Info(a) => info(&a, out, err)?, + Command::Convert(a) => convert(&a, out)?, + Command::Run(a) => run_cmds(&a, out, err)?, + Command::Script(a) => script(&a, out, err)?, + Command::Commands(a) => commands(&a, out), + Command::Describe(a) => describe(&a, out)?, + Command::App(a) => app(&a, out)?, + Command::Mcp(a) => mcp(&a, err)?, + Command::Parity(a) => parity(&a, out)?, + Command::Plugins => { + for p in soundcraft_dsp::plugins() { + outln!(out, "{:<20} {:<22} {:?}{}", p.id, p.name, p.category, if p.is_instrument { " (instrument)" } else { "" }); + } + } + } + Ok(()) +} diff --git a/apps/soundcraft-cli/src/main.rs b/apps/soundcraft-cli/src/main.rs index 899ba71e..7d381be7 100644 --- a/apps/soundcraft-cli/src/main.rs +++ b/apps/soundcraft-cli/src/main.rs @@ -1,684 +1,12 @@ -//! `soundcraft-cli`: SoundCraft without a window. -//! -//! ```text -//! soundcraft-cli info FILE audio file or .scraft summary -//! soundcraft-cli convert IN OUT [--bit-depth 16|24|32f] [--rate HZ] [--normalize] [--dither | --no-dither] -//! dithers only when the output loses resolution, unless told -//! soundcraft-cli run [--in S.scraft | --demo] [--cmd ID[=JSON]]… [--bounce OUT] [--start T --end T] [--save S.scraft] [--inspect] -//! soundcraft-cli script FILE [--in S | --demo] [--save S] [--bounce OUT [--start T --end T]] (one `ID JSON` per line; -//! without --start/--end, --bounce covers the whole session) -//! soundcraft-cli commands [FILTER] list commands -//! soundcraft-cli describe ID one command's details -//! soundcraft-cli app [--port P] METHOD [JSON] call a running app's control channel -//! soundcraft-cli mcp [--connect PORT] [--demo] [--automation-read-root DIR]... [--automation-write-root DIR]... -//! MCP server on stdio. The roots confine every file the -//! agent's commands read (open, import, presets) and write -//! (save, bounce, export) to those folders, links followed; -//! each flag may be given more than once, a path inside any -//! of its folders being allowed, and a kind left out grants -//! none of its access. They run the in-process server, so -//! they conflict with --connect. -//! soundcraft-cli parity [--write PATH] parity report (markdown) -//! soundcraft-cli plugins list built-in plugins -//! ``` +//! `soundcraft-cli`: SoundCraft without a window. See the library crate for the commands. + #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] -mod automation_args; - -use serde_json::{Value, json}; -use soundcraft_automation::{Backend, Headless, Remote, mcp::Server}; -use soundcraft_engine::Engine; -use soundcraft_engine::file_access::{self, AutomationRoots}; -use std::io::Write; -use std::process::ExitCode; - -/// Print without panicking on a closed pipe. A write error must not skip later work -/// such as `--bounce` or `--save`, and must not by itself report success. -macro_rules! outln { - ($($t:tt)*) => {{ - let mut o = std::io::stdout().lock(); - let _ = writeln!(o, $($t)*); - }}; -} - -fn arg_value(args: &[String], flag: &str) -> Option { - args.iter().enumerate().find_map(|(i, a)| { - if a == flag { args.get(i + 1).cloned() } else { a.strip_prefix(flag).and_then(|rest| rest.strip_prefix('=')).map(str::to_owned) } - }) -} - -/// Every option a subcommand accepts, with whether it carries a value. -type Options = [(&'static str, bool)]; - -/// A misspelled option, or one whose value is missing, reads as absent through `arg_value`, so the -/// subcommand would run without what was asked for and still report success. Reject both instead. -fn check_options(args: &[String], allowed: &Options) -> Result<(), String> { - let mut i = 0; - while i < args.len() { - let Some(name) = args[i].strip_prefix("--") else { - i += 1; - continue; - }; - let (name, inline) = name.split_once('=').map_or((name, None), |(n, v)| (n, Some(v))); - let flag = format!("--{name}"); - let Some(&(_, takes_value)) = allowed.iter().find(|(f, _)| f == &flag) else { - return Err(format!("unknown option {flag}")); - }; - match (takes_value, inline) { - (false, Some(_)) => return Err(format!("{flag} takes no value")), - (true, None) => { - // A following option is the next flag, not this one's value. - if args.get(i + 1).is_none_or(|v| v.starts_with("--")) { - return Err(format!("{flag} needs a value")); - } - i += 1; - } - _ => {} - } - i += 1; - } - Ok(()) -} - -/// `--in`, `--demo`/`--sample`, `--bounce`, `--start`, `--end`, `--save` and `--inspect`, read by -/// `load_engine` and `finish` and so shared by `run` and `script`. -const ENGINE_OPTIONS: &Options = &[ - ("--in", true), - ("--demo", false), - ("--sample", false), - ("--bounce", true), - ("--start", true), - ("--end", true), - ("--save", true), - ("--inspect", false), -]; - -fn fail(msg: impl std::fmt::Display) -> ExitCode { - eprintln!("soundcraft-cli: {msg}"); - ExitCode::FAILURE -} - -/// The usage block of the module doc comment, up to its closing code fence. -fn help_text() -> String { - include_str!("main.rs") - .lines() - .skip(3) - .take_while(|l| l.trim_end() != "//! ```") - .map(|l| l.trim_start_matches("//! ")) - .collect::>() - .join("\n") -} - -fn main() -> ExitCode { - let args: Vec = std::env::args().skip(1).collect(); - let Some(cmd) = args.first().cloned() else { - eprintln!("usage: soundcraft-cli … (see --help)"); - return ExitCode::FAILURE; - }; - let rest: Vec = args.iter().skip(1).cloned().collect(); - let code = match cmd.as_str() { - "--version" | "-V" | "version" => { - outln!("SoundCraft CLI {}", env!("CARGO_PKG_VERSION")); - ExitCode::SUCCESS - } - "--help" | "-h" | "help" => { - outln!("{}", help_text()); - ExitCode::SUCCESS - } - "info" => info(&rest), - "convert" => convert(&rest), - "run" => run(&rest), - "script" => script(&rest), - "commands" => commands(&rest), - "describe" => describe(&rest), - "app" => app(&rest), - "mcp" => mcp(&rest), - "parity" => parity(&rest), - "plugins" => { - for p in soundcraft_dsp::plugins() { - outln!("{:<20} {:<22} {:?}{}", p.id, p.name, p.category, if p.is_instrument { " (instrument)" } else { "" }); - } - ExitCode::SUCCESS - } - other => fail(format!("unknown subcommand `{other}`")), - }; - // The subcommand's engine and its plugin instances are gone by now. +fn main() { + // `args_os`, not `args`: a non-Unicode argument must reach clap, not panic before anything is printed. + let code = soundcraft_cli::run(std::env::args_os().skip(1), &mut std::io::stdout(), &mut std::io::stderr()); + // The subcommand's engine and its plugin instances are gone by now. This is process teardown, + // so it lives here and not in `run`, which tests call many times in one process. soundcraft_engine::shutdown_plugin_hosts(); - code -} - -fn info(args: &[String]) -> ExitCode { - let Some(path) = args.first() else { return fail("info FILE") }; - if path.ends_with(".scraft") { - let mut e = Engine::default(); - match soundcraft_engine::io::open_session(&mut e, path) { - Ok(missing) => warn_missing(&missing), - Err(err) => return fail(err), - } - outln!("{}", soundcraft_engine::inspect::session_text(&e)); - return ExitCode::SUCCESS; - } - let bytes = match std::fs::read(path) { - Ok(b) => b, - Err(e) => return fail(format!("{path}: {e}")), - }; - let ext = std::path::Path::new(path).extension().and_then(std::ffi::OsStr::to_str); - match soundcraft_audio_io::probe(&bytes, ext) { - Ok(i) => { - outln!("{}", serde_json::to_string_pretty(&json!({"format": i.format, "sample_format": i.sample_format, "sample_rate": i.sample_rate, "channels": i.channels, "frames": i.frames, "seconds": i.duration_secs(), "bwf": i.bwf})).unwrap_or_default()); - ExitCode::SUCCESS - } - Err(e) => fail(format!("{path}: {e}")), - } -} - -fn convert(args: &[String]) -> ExitCode { - let (Some(input), Some(output)) = (args.first(), args.get(1)) else { return fail("convert IN OUT") }; - const OPTIONS: &Options = &[("--rate", true), ("--bit-depth", true), ("--normalize", false), ("--dither", false), ("--no-dither", false)]; - if let Err(e) = check_options(&args[2..], OPTIONS) { - return fail(e); - } - let rate = match parse_rate(arg_value(args, "--rate").as_deref()) { - Ok(r) => r, - Err(e) => return fail(e), - }; - let bit_depth = match parse_bit_depth(arg_value(args, "--bit-depth").as_deref()) { - Ok(b) => b, - Err(e) => return fail(e), - }; - let dither_flag = match parse_dither(args) { - Ok(d) => d, - Err(e) => return fail(e), - }; - let bytes = match std::fs::read(input) { - Ok(b) => b, - Err(e) => return fail(format!("{input}: {e}")), - }; - let ext = std::path::Path::new(input).extension().and_then(std::ffi::OsStr::to_str); - let (info, mut buf) = match soundcraft_audio_io::decode(&bytes, ext) { - Ok(x) => x, - Err(e) => return fail(format!("{input}: {e}")), - }; - if let Some(rate) = rate { - buf.channels = soundcraft_dsp::offline::resample(&buf.channels, buf.sample_rate, rate); - buf.sample_rate = rate; - } - let normalize = args.iter().any(|a| a == "--normalize"); - if normalize { - soundcraft_dsp::offline::normalize(&mut buf.channels, -0.1, false); - } - let processed = rate.is_some_and(|r| r != info.sample_rate) || normalize; - let dither = dither_flag.unwrap_or_else(|| auto_dither(info.sample_format, bit_depth, processed)); - let out_ext = std::path::Path::new(output).extension().and_then(std::ffi::OsStr::to_str).unwrap_or("wav").to_ascii_lowercase(); - let Some(format) = soundcraft_audio_io::encode_format_for(&out_ext) else { - return fail(format!("{output}: cannot write `.{out_ext}` files (supported: {})", soundcraft_audio_io::ENCODE_EXTENSIONS)); - }; - let opts = soundcraft_audio_io::EncodeOptions { format, bit_depth, dither, bwf: None }; - match soundcraft_audio_io::encode(&buf, &opts).map_err(|e| e.to_string()).and_then(|b| std::fs::write(output, b).map_err(|e| e.to_string())) { - Ok(()) => { - outln!("wrote {output} ({} ch, {} Hz, {:.2} s)", buf.num_channels(), buf.sample_rate, buf.duration_secs()); - ExitCode::SUCCESS - } - Err(e) => fail(e), - } -} - -fn parse_rate(v: Option<&str>) -> Result, String> { - match v { - None => Ok(None), - Some(r) => match r.parse::() { - Ok(hz) if hz > 0 => Ok(Some(hz)), - _ => Err(format!("--rate: expected a sample rate in Hz, got `{r}`")), - }, - } -} - -fn parse_bit_depth(v: Option<&str>) -> Result { - match v { - None | Some("24") => Ok(soundcraft_audio_io::BitDepth::Int24), - Some("16") => Ok(soundcraft_audio_io::BitDepth::Int16), - Some("32") | Some("32f") => Ok(soundcraft_audio_io::BitDepth::Float32), - Some(b) => Err(format!("--bit-depth: expected 16, 24 or 32f, got `{b}`")), - } -} - -/// `--dither` / `--no-dither`: `Some` when the caller chose, `None` to decide from the depths. -fn parse_dither(args: &[String]) -> Result, String> { - match (args.iter().any(|a| a == "--dither"), args.iter().any(|a| a == "--no-dither")) { - (true, true) => Err("--dither and --no-dither: choose one".into()), - (true, false) => Ok(Some(true)), - (false, true) => Ok(Some(false)), - (false, false) => Ok(None), - } -} - -/// Whether `convert` dithers when not told: only when writing integer samples that cannot hold the -/// source exactly, i.e. the output has fewer bits than the source, the source is float or lossy, or -/// resampling/normalizing produced new sample values. A 16-bit file written as 16 or 24-bit, or -/// anything written as 32-bit float, is copied sample for sample (digital silence stays silent). -fn auto_dither(source: soundcraft_audio_io::SampleFormat, target: soundcraft_audio_io::BitDepth, processed: bool) -> bool { - use soundcraft_audio_io::{BitDepth, SampleFormat}; - let target_bits = match target { - BitDepth::Float32 => return false, - BitDepth::Int16 => 16, - BitDepth::Int24 => 24, - BitDepth::Int32 => 32, - }; - if processed { - return true; - } - match source { - SampleFormat::Int8 => target_bits < 8, - SampleFormat::Int16 => target_bits < 16, - SampleFormat::Int24 => target_bits < 24, - // Decoded through f32, which holds 24 bits: a 32-bit integer source has already lost - // resolution, so treat it like float. - SampleFormat::Int32 | SampleFormat::Float32 | SampleFormat::Float64 | SampleFormat::Compressed => true, - } -} - -/// One stderr line per media file a session refers to but could not load (the exit code is unchanged). -fn missing_warnings(missing: &[String]) -> Vec { - missing.iter().map(|m| format!("warning: missing media: {m}")).collect() -} - -fn warn_missing(missing: &[String]) { - for line in missing_warnings(missing) { - eprintln!("{line}"); - } -} - -fn load_engine(args: &[String]) -> Result { - if let Some(p) = arg_value(args, "--in") { - let mut e = Engine::default(); - let missing = soundcraft_engine::io::open_session(&mut e, &p).map_err(|e| e.to_string())?; - warn_missing(&missing); - Ok(e) - } else if args.iter().any(|a| a == "--demo" || a == "--sample") { - Ok(soundcraft_engine::demo::demo_engine()) - } else { - Ok(Engine::default()) - } -} - -fn finish(e: &mut Engine, args: &[String]) -> ExitCode { - if let Some(out) = arg_value(args, "--bounce") { - let mut p = json!({"path": out}); - if let Some(s) = arg_value(args, "--start") { - p["start"] = time_arg(&s); - } - if let Some(s) = arg_value(args, "--end") { - p["end"] = time_arg(&s); - } - // No range given: bounce the whole session, not whatever edit selection the commands left. - if p.get("start").is_none() && p.get("end").is_none() { - p["start"] = json!(0); - p["end"] = json!(e.session().content_end().max(1)); - } - match e.execute("file.bounce_mix", &p) { - Ok(v) => outln!("{v}"), - Err(err) => return fail(err), - } - } - if let Some(path) = arg_value(args, "--save") { - match e.execute("session.save", &json!({"path": path, "replace": true})) { - Ok(v) => outln!("{v}"), - Err(err) => return fail(err), - } - } - if args.iter().any(|a| a == "--inspect") { - outln!("{}", serde_json::to_string_pretty(&soundcraft_engine::inspect::session(e, false)).unwrap_or_default()); - } - ExitCode::SUCCESS -} - -/// `12.5` → seconds; anything else is passed through as a time string. -fn time_arg(s: &str) -> Value { - s.parse::().map_or_else(|_| json!(s), |f| json!({"seconds": f})) -} - -/// `ID` or `ID=JSON` → command id and params; malformed JSON is an error, not `{}`. -fn parse_cmd_spec(spec: &str) -> Result<(String, Value), String> { - match spec.split_once('=') { - Some((id, p)) => serde_json::from_str::(p).map(|v| (id.to_string(), v)).map_err(|err| format!("{id}: bad JSON: {err}")), - None => Ok((spec.to_string(), json!({}))), - } -} - -fn run(args: &[String]) -> ExitCode { - let options: Vec<(&str, bool)> = ENGINE_OPTIONS.iter().copied().chain([("--cmd", true)]).collect(); - if let Err(e) = check_options(args, &options) { - return fail(e); - } - let mut e = match load_engine(args) { - Ok(e) => e, - Err(err) => return fail(err), - }; - let mut it = args.iter(); - while let Some(a) = it.next() { - let spec = match a.strip_prefix("--cmd=") { - Some(inline) => inline, - None if a == "--cmd" => match it.next() { - Some(next) => next, - None => continue, - }, - None => continue, - }; - let (id, params) = match parse_cmd_spec(spec) { - Ok(v) => v, - Err(err) => return fail(err), - }; - match e.execute(&id, ¶ms) { - Ok(v) => outln!("{id}: {v}"), - Err(err) => return fail(format!("{id}: {err}")), - } - } - finish(&mut e, args) -} - -fn script(args: &[String]) -> ExitCode { - let Some(path) = args.first() else { return fail("script FILE") }; - let options: Vec<(&str, bool)> = ENGINE_OPTIONS.iter().copied().chain([("--keep-going", false)]).collect(); - if let Err(e) = check_options(&args[1..], &options) { - return fail(e); - } - let text = match std::fs::read_to_string(path) { - Ok(t) => t, - Err(e) => return fail(format!("{path}: {e}")), - }; - let mut e = match load_engine(args) { - Ok(e) => e, - Err(err) => return fail(err), - }; - let keep = args.iter().any(|a| a == "--keep-going"); - for (n, line) in text.lines().enumerate() { - let line = line.trim(); - if line.is_empty() || line.starts_with('#') { - continue; - } - let (id, p) = line.split_once(char::is_whitespace).unwrap_or((line, "{}")); - let params = match serde_json::from_str::(p.trim()) { - Ok(v) => v, - Err(err) => return fail(format!("{path}:{}: bad JSON: {err}", n + 1)), - }; - match e.execute(id, ¶ms) { - Ok(v) => outln!("{id}: {v}"), - Err(err) => { - eprintln!("{path}:{}: {id}: {err}", n + 1); - if !keep { - return ExitCode::FAILURE; - } - } - } - } - finish(&mut e, args) -} - -fn commands(args: &[String]) -> ExitCode { - let filter = args.first().cloned().unwrap_or_default(); - let e = Engine::default(); - for c in soundcraft_engine::command_specs() { - if !filter.is_empty() && !c.id.contains(&filter) && !c.label.to_ascii_lowercase().contains(&filter.to_ascii_lowercase()) { - continue; - } - let i = c.info(&e); - outln!("{:<36} {:<34} {}", i.id, i.label, i.menu.join(" > ")); - } - ExitCode::SUCCESS -} - -fn describe(args: &[String]) -> ExitCode { - let Some(id) = args.first() else { return fail("describe ID") }; - match soundcraft_engine::find_command(id) { - Some(c) => { - outln!("{}", serde_json::to_string_pretty(&c.info(&Engine::default())).unwrap_or_default()); - ExitCode::SUCCESS - } - None => fail(format!("unknown command `{id}`")), - } -} - -/// Control-channel methods. Every other dotted id is an engine command. -fn is_control_method(method: &str) -> bool { - matches!( - method, - "engine.execute" - | "command" - | "engine.commands" - | "engine.parity" - | "session.inspect" - | "document.inspect" - | "ui.inspect" - | "ui.menu.list" - | "ui.menu.invoke" - | "ui.set" - | "ui.move" - | "ui.click" - | "ui.drag" - | "ui.key" - | "ui.text" - | "ui.screenshot" - | "app.quit" - ) -} - -/// A command id is shorthand for `engine.execute`. Documented control methods stay methods. -fn app_request(method: &str, params: Value) -> (String, Value) { - if is_control_method(method) || !method.contains('.') { - (method.to_string(), params) - } else { - ("engine.execute".to_string(), json!({"command": method, "params": params})) - } -} - -fn app(args: &[String]) -> ExitCode { - let port = arg_value(args, "--port").or_else(|| std::env::var("SOUNDCRAFT_CONTROL_PORT").ok()).unwrap_or_else(|| "7979".into()); - let positional: Vec<&String> = args - .iter() - .enumerate() - .filter(|(i, a)| !(a.starts_with("--") || (*i > 0 && args.get(i - 1).is_some_and(|p| p == "--port")))) - .map(|(_, a)| a) - .collect(); - let Some(method) = positional.first() else { return fail("app [--port P] METHOD [JSON]") }; - let params = match positional.get(1) { - Some(p) => match serde_json::from_str::(p) { - Ok(v) => v, - Err(err) => return fail(format!("{method}: bad JSON: {err}")), - }, - None => json!({}), - }; - let mut r = Remote::new(&port); - // A bare command id is shorthand for engine.execute. Control-channel methods stay methods. - let (m, p) = app_request(method, params); - match r.call(&m, p) { - Ok(v) => { - outln!("{}", serde_json::to_string_pretty(&v).unwrap_or_default()); - ExitCode::SUCCESS - } - Err(e) => fail(e), - } -} - -/// What `mcp` was asked for. -struct McpArgs { - connect: Option, - demo: bool, - roots: Option>, -} - -/// Parse the `mcp` options up front: a malformed or misspelled root flag must not leave the -/// server running unconfined. -fn parse_mcp_args(args: &[String]) -> Result { - let mut connect = None; - let mut demo = false; - let mut roots_args = automation_args::AutomationArgs::default(); - let mut it = args.iter().cloned(); - while let Some(a) = it.next() { - if roots_args.consume(&a, &mut it)? { - continue; - } - match a.as_str() { - "--connect" => connect = Some(it.next().ok_or("--connect needs a port or host:port")?), - "--demo" => demo = true, - other if other.starts_with("--automation-") => return Err(format!("unknown mcp option `{other}`")), - _ => {} - } - } - let read: Vec<&std::path::Path> = roots_args.read_roots.iter().map(std::path::PathBuf::as_path).collect(); - let write: Vec<&std::path::Path> = roots_args.write_roots.iter().map(std::path::PathBuf::as_path).collect(); - let roots = AutomationRoots::new(&read, &write)?; - if roots.is_some() && connect.is_some() { - return Err("--automation-read-root and --automation-write-root confine the in-process server; a running app is not confined by them, \ -so drop --connect (or use --connect without the roots)" - .into()); - } - Ok(McpArgs { connect, demo, roots }) -} - -fn mcp(args: &[String]) -> ExitCode { - let McpArgs { connect, demo, roots } = match parse_mcp_args(args) { - Ok(a) => a, - Err(err) => return fail(err), - }; - let stdin = std::io::stdin(); - let stdout = std::io::stdout(); - let r = if let Some(port) = connect { - let mut s = Server::new(Remote::new(&port)); - s.serve(stdin.lock(), stdout.lock()) - } else { - // `--demo` is the operator's own choice, made before the roots come into force. - let e = if demo { soundcraft_engine::demo::demo_engine() } else { Engine::default() }; - let mut h = Headless::new(e); - if let Some(roots) = roots { - // Every thread of this process, not only the server's: nothing reaches past the roots. - file_access::confine_process(roots.clone()); - eprintln!("soundcraft-cli: files confined: {}", roots.describe()); - h = h.with_automation_roots(Some(roots)); - } - let mut s = Server::new(h); - s.serve(stdin.lock(), stdout.lock()) - }; - match r { - Ok(()) => ExitCode::SUCCESS, - Err(e) => fail(e), - } -} - -fn parity(args: &[String]) -> ExitCode { - let p = soundcraft_engine::catalog::parity_json(); - let mut md = String::new(); - md.push_str("# Parity report\n\nGenerated by `soundcraft-cli parity --write docs/parity-checklist.md` (also `cargo xtask parity`).\n"); - md.push_str("It compares the incumbent's menu tree (feature names observed black-box, `crates/engine/catalog/menus.txt`) with SoundCraft's command registry.\n"); - md.push_str("UI-layer commands (windows, AudioSuite) are counted by the app's own report (`engine.parity` over the control channel).\n\n"); - md.push_str(&format!( - "**Menu items implemented by the engine: {} / {} ({}%)**\n\n| Menu | Implemented | Total |\n|---|---:|---:|\n", - p["implemented"], p["total"], p["percent"] - )); - for m in p["per_menu"].as_array().cloned().unwrap_or_default() { - md.push_str(&format!("| {} | {} | {} |\n", m["menu"].as_str().unwrap_or(""), m["implemented"], m["total"])); - } - md.push_str("\n## Not yet implemented\n\n"); - for m in p["missing"].as_array().cloned().unwrap_or_default() { - md.push_str(&format!("- {}\n", m.as_str().unwrap_or(""))); - } - if let Some(path) = arg_value(args, "--write") { - if let Err(e) = std::fs::write(&path, &md) { - return fail(format!("{path}: {e}")); - } - outln!("wrote {path}: {}% ({}/{})", p["percent"], p["implemented"], p["total"]); - } else { - outln!("{md}"); - } - ExitCode::SUCCESS -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn command_ids_are_execute_shorthand_and_control_methods_stay_methods() { - let (m, p) = app_request("engine.history", json!({})); - assert_eq!(m, "engine.execute"); - assert_eq!(p["command"], "engine.history"); - let (m, p) = app_request("ui.theme", json!({"mode": "light"})); - assert_eq!(m, "engine.execute"); - assert_eq!(p["params"]["mode"], "light"); - let (m, _) = app_request("session.new", json!({"name": "Session A"})); - assert_eq!(m, "engine.execute"); - let (m, _) = app_request("track.new", json!({})); - assert_eq!(m, "engine.execute"); - for method in ["session.inspect", "engine.commands", "engine.parity", "ui.screenshot", "app.quit"] { - assert_eq!(app_request(method, json!({})).0, method); - } - } - - #[test] - fn convert_rejects_invalid_rate_and_bit_depth() { - assert_eq!(parse_rate(None), Ok(None)); - assert_eq!(parse_rate(Some("44100")), Ok(Some(44_100))); - assert!(parse_rate(Some("4410x")).is_err()); - assert!(parse_rate(Some("0")).is_err()); - assert!(matches!(parse_bit_depth(None), Ok(soundcraft_audio_io::BitDepth::Int24))); - assert!(matches!(parse_bit_depth(Some("16")), Ok(soundcraft_audio_io::BitDepth::Int16))); - assert!(matches!(parse_bit_depth(Some("32f")), Ok(soundcraft_audio_io::BitDepth::Float32))); - assert!(parse_bit_depth(Some("8")).is_err()); - } - - #[test] - fn convert_dither_flags() { - let args = |a: &[&str]| a.iter().map(|s| s.to_string()).collect::>(); - assert_eq!(parse_dither(&args(&["in.wav", "out.wav"])), Ok(None)); - assert_eq!(parse_dither(&args(&["in.wav", "out.wav", "--dither"])), Ok(Some(true))); - assert_eq!(parse_dither(&args(&["in.wav", "out.wav", "--no-dither"])), Ok(Some(false))); - assert!(parse_dither(&args(&["in.wav", "out.wav", "--dither", "--no-dither"])).is_err()); - } - - #[test] - fn convert_dithers_only_when_resolution_is_lost() { - use soundcraft_audio_io::{BitDepth, SampleFormat}; - // Same depth or wider: the samples fit exactly. - assert!(!auto_dither(SampleFormat::Int16, BitDepth::Int16, false)); - assert!(!auto_dither(SampleFormat::Int16, BitDepth::Int24, false)); - assert!(!auto_dither(SampleFormat::Int24, BitDepth::Int24, false)); - assert!(!auto_dither(SampleFormat::Int8, BitDepth::Int16, false)); - // Float output never dithers. - assert!(!auto_dither(SampleFormat::Float32, BitDepth::Float32, true)); - assert!(!auto_dither(SampleFormat::Int24, BitDepth::Float32, false)); - // Fewer bits, a float or lossy source, or processed samples: dither. - assert!(auto_dither(SampleFormat::Int24, BitDepth::Int16, false)); - assert!(auto_dither(SampleFormat::Float32, BitDepth::Int24, false)); - assert!(auto_dither(SampleFormat::Compressed, BitDepth::Int16, false)); - assert!(auto_dither(SampleFormat::Int32, BitDepth::Int24, false)); - assert!(auto_dither(SampleFormat::Int16, BitDepth::Int16, true)); - } - - #[test] - fn missing_media_gets_one_warning_each() { - assert!(missing_warnings(&[]).is_empty()); - let w = missing_warnings(&["a.wav".to_string(), "b.wav: decode failed".to_string()]); - assert_eq!(w, vec!["warning: missing media: a.wav", "warning: missing media: b.wav: decode failed"]); - } - - #[test] - fn cmd_spec_parses_id_and_json() { - assert_eq!(parse_cmd_spec("a.b").unwrap(), ("a.b".to_string(), json!({}))); - assert_eq!(parse_cmd_spec("a.b={\"x\":1}").unwrap(), ("a.b".to_string(), json!({"x": 1}))); - } - - #[test] - fn cmd_spec_rejects_truncated_json() { - let err = parse_cmd_spec("a.b={\"x\":").unwrap_err(); - assert!(err.starts_with("a.b: bad JSON"), "{err}"); - } - - #[test] - fn help_text_is_only_the_usage_block() { - let help = help_text(); - assert!(help.starts_with("soundcraft-cli info FILE")); - assert!(help.ends_with("soundcraft-cli plugins list built-in plugins")); - assert!(help.contains("--automation-read-root DIR") && help.contains("--automation-write-root DIR")); - assert!(!help.contains("```")); - assert!(!help.contains("#![") && !help.contains("use serde_json")); - } + std::process::exit(code); } diff --git a/apps/soundcraft-cli/src/mcp.rs b/apps/soundcraft-cli/src/mcp.rs new file mode 100644 index 00000000..874c85ec --- /dev/null +++ b/apps/soundcraft-cli/src/mcp.rs @@ -0,0 +1,46 @@ +use super::args::McpArgs; +use super::error::CliError; +use soundcraft_automation::{ControlTarget, Headless, Remote, mcp::Server}; +use soundcraft_engine::Engine; +use soundcraft_engine::file_access::{self, AutomationRoots}; +use std::io::Write; + +/// What `mcp` serves. +struct Setup { + connect: Option, + demo: bool, + roots: Option>, +} + +/// Open the roots before the server starts: a folder that can't be opened must not leave the +/// server running unconfined. +fn setup(args: &McpArgs) -> Result { + let read: Vec<&std::path::Path> = args.automation.automation_read_root.iter().map(std::path::PathBuf::as_path).collect(); + let write: Vec<&std::path::Path> = args.automation.automation_write_root.iter().map(std::path::PathBuf::as_path).collect(); + let roots = AutomationRoots::new(&read, &write)?; + Ok(Setup { connect: args.connect.clone(), demo: args.demo, roots }) +} + +/// Serves on the process's real stdin and stdout; only the notes go to `err`. +pub(crate) fn mcp(args: &McpArgs, err: &mut dyn Write) -> Result<(), CliError> { + let Setup { connect, demo, roots } = setup(args)?; + let stdin = std::io::stdin(); + let stdout = std::io::stdout(); + let served = if let Some(target) = connect { + let mut s = Server::new(Remote::new(&target)); + s.serve(stdin.lock(), stdout.lock()) + } else { + // `--demo` is the operator's own choice, made before the roots come into force. + let e = if demo { soundcraft_engine::demo::demo_engine() } else { Engine::default() }; + let mut h = Headless::new(e); + if let Some(roots) = roots { + // Every thread of this process, not only the server's: nothing reaches past the roots. + file_access::confine_process(roots.clone()); + outln!(err, "soundcraft-cli: files confined: {}", roots.describe()); + h = h.with_automation_roots(Some(roots)); + } + let mut s = Server::new(h); + s.serve(stdin.lock(), stdout.lock()) + }; + served.map_err(CliError::Serve) +} diff --git a/apps/soundcraft-cli/src/output.rs b/apps/soundcraft-cli/src/output.rs new file mode 100644 index 00000000..f6384222 --- /dev/null +++ b/apps/soundcraft-cli/src/output.rs @@ -0,0 +1,32 @@ +/// Print to a writer without panicking on a closed pipe. A write error must not skip later work +/// such as `--bounce` or `--save`, and must not by itself report success. +macro_rules! outln { + ($w:expr, $($t:tt)*) => {{ + let _ = writeln!($w, $($t)*); + }}; +} + +/// Print `value` as JSON: pretty-printed, or on one line when `compact`. Every subcommand that +/// prints a JSON document goes through here, so `--compact` means the same thing everywhere. +pub(crate) fn print_json(out: &mut dyn std::io::Write, value: &impl serde::Serialize, compact: bool) { + let text = if compact { serde_json::to_string(value) } else { serde_json::to_string_pretty(value) }; + outln!(out, "{}", text.unwrap_or_default()); +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn compact_is_one_line_and_pretty_is_the_same_value_over_several() { + let v = json!({"a": [1, 2], "b": {"c": "d"}}); + let (mut pretty, mut compact) = (Vec::new(), Vec::new()); + print_json(&mut pretty, &v, false); + print_json(&mut compact, &v, true); + let (pretty, compact) = (String::from_utf8(pretty).unwrap_or_default(), String::from_utf8(compact).unwrap_or_default()); + assert!(pretty.trim_end().lines().count() > 1); + assert_eq!(compact, "{\"a\":[1,2],\"b\":{\"c\":\"d\"}}\n"); + assert_eq!(serde_json::from_str::(&pretty).ok(), Some(v)); + } +} diff --git a/apps/soundcraft-cli/src/session.rs b/apps/soundcraft-cli/src/session.rs new file mode 100644 index 00000000..20e17b1a --- /dev/null +++ b/apps/soundcraft-cli/src/session.rs @@ -0,0 +1,111 @@ +use super::args::{CmdSpec, EngineArgs, RunArgs, ScriptArgs}; +use super::error::{CliError, unicode}; +use super::output::print_json; +use serde_json::{Value, json}; +use soundcraft_engine::Engine; +use std::io::Write; + +/// One stderr line per media file a session refers to but could not load (the exit code is unchanged). +pub(crate) fn warn_missing(err: &mut dyn Write, missing: &[String]) { + for m in missing { + outln!(err, "warning: missing media: {m}"); + } +} + +/// The outputs go to the engine as JSON text, so a non-Unicode one is refused before any command runs. +fn load_engine(args: &EngineArgs, err: &mut dyn Write) -> Result { + if let Some(bounce) = &args.bounce { + unicode(bounce)?; + } + if let Some(save) = args.save_path() { + unicode(&save)?; + } + if let Some(p) = &args.input { + let mut e = Engine::default(); + let missing = soundcraft_engine::io::open_session(&mut e, p)?; + warn_missing(err, &missing); + Ok(e) + } else if args.demo { + Ok(soundcraft_engine::demo::demo_engine()) + } else { + Ok(Engine::default()) + } +} + +fn finish(e: &mut Engine, args: &EngineArgs, out: &mut dyn Write) -> Result<(), CliError> { + if let Some(path) = &args.bounce { + let mut p = json!({"path": unicode(path)?}); + if let Some(s) = &args.start { + p["start"] = s.to_json(); + } + if let Some(s) = &args.end { + p["end"] = s.to_json(); + } + // No range given: bounce the whole session, not whatever edit selection the commands left. + if p.get("start").is_none() && p.get("end").is_none() { + p["start"] = json!(0); + p["end"] = json!(e.session().content_end().max(1)); + } + let v = e.execute("file.bounce_mix", &p)?; + outln!(out, "{v}"); + } + if let Some(path) = args.save_path() { + let v = e.execute("session.save", &json!({"path": unicode(&path)?, "replace": true}))?; + outln!(out, "{v}"); + } + if args.inspect { + print_json(out, &soundcraft_engine::inspect::session(e, false), args.compact); + } + Ok(()) +} + +pub(crate) fn run_cmds(args: &RunArgs, out: &mut dyn Write, err: &mut dyn Write) -> Result<(), CliError> { + let mut e = load_engine(&args.engine, err)?; + for CmdSpec { id, params } in &args.cmd { + match e.execute(id, params) { + Ok(v) => outln!(out, "{id}: {v}"), + Err(source) => return Err(CliError::Command { id: id.clone(), source }), + } + } + finish(&mut e, &args.engine, out) +} + +pub(crate) fn script(args: &ScriptArgs, out: &mut dyn Write, err: &mut dyn Write) -> Result<(), CliError> { + let path = &args.file; + let text = std::fs::read_to_string(path).map_err(|source| CliError::File { path: path.clone(), source })?; + let mut e = load_engine(&args.engine, err)?; + let keep = args.keep_going; + for (n, line) in text.lines().enumerate() { + let line = line.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + let (id, p) = line.split_once(char::is_whitespace).unwrap_or((line, "{}")); + let params = serde_json::from_str::(p.trim()).map_err(|source| CliError::ScriptJson { path: path.clone(), line: n + 1, source })?; + match e.execute(id, ¶ms) { + Ok(v) => outln!(out, "{id}: {v}"), + Err(source) => { + let failure = CliError::ScriptLine { path: path.clone(), line: n + 1, id: id.to_owned(), source }; + if !keep { + return Err(failure); + } + outln!(err, "{failure}"); + } + } + } + finish(&mut e, &args.engine, out) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn missing_media_gets_one_warning_each() { + let mut err = Vec::new(); + warn_missing(&mut err, &[]); + assert!(err.is_empty()); + warn_missing(&mut err, &["a.wav".to_string(), "b.wav: decode failed".to_string()]); + assert_eq!(String::from_utf8(err).unwrap(), "warning: missing media: a.wav\nwarning: missing media: b.wav: decode failed\n"); + } +} diff --git a/apps/soundcraft-cli/tests/it/behaviour.rs b/apps/soundcraft-cli/tests/it/behaviour.rs new file mode 100644 index 00000000..ce0076d1 --- /dev/null +++ b/apps/soundcraft-cli/tests/it/behaviour.rs @@ -0,0 +1,550 @@ +//! How `soundcraft-cli` parses its arguments and reports results today, run as the real binary. +//! A change to the command line's behaviour updates them on purpose. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; + +use soundcraft_audio_io::{AudioBuffer, BitDepth, EncodeOptions, FileFormat}; + +/// What one run of the binary produced. +struct Invoke { + code: i32, + stdout: String, + stderr: String, +} + +/// Runs the binary in `dir` with empty stdin and no control-port variable. +fn invoke(dir: &Path, args: &[&str]) -> Invoke { + invoke_env(dir, args, None) +} + +fn invoke_env(dir: &Path, args: &[&str], control_port: Option<&str>) -> Invoke { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_soundcraft-cli")); + cmd.env("SOUNDCRAFT_NO_PREFS", "1").env_remove("SOUNDCRAFT_CONTROL_PORT"); + if let Some(port) = control_port { + cmd.env("SOUNDCRAFT_CONTROL_PORT", port); + } + let out = cmd.current_dir(dir).args(args).stdin(Stdio::null()).output().unwrap(); + Invoke { code: out.status.code().unwrap(), stdout: String::from_utf8(out.stdout).unwrap(), stderr: String::from_utf8(out.stderr).unwrap() } +} + +/// A fresh directory holding `in.wav` (200 samples, 16-bit mono, 48 kHz) and `s.txt` (a one-command script). +fn sandbox(tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("sc-cli-behaviour-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let mut buf = AudioBuffer::new(48_000, 1, 200); + for (i, d) in buf.channels[0].iter_mut().enumerate() { + *d = i as f32 / 400.0; + } + let opts = EncodeOptions { format: FileFormat::Wav, bit_depth: BitDepth::Int16, dither: false, bwf: None }; + std::fs::write(dir.join("in.wav"), soundcraft_audio_io::encode(&buf, &opts).unwrap()).unwrap(); + std::fs::write(dir.join("s.txt"), "track.new {}\n").unwrap(); + dir +} + +/// The sample rate of the WAV `name` in `dir`. +fn rate_of(dir: &Path, name: &str) -> u32 { + soundcraft_audio_io::probe(&std::fs::read(dir.join(name)).unwrap(), None).unwrap().sample_rate +} + +/// Bit depth of the WAV `name` in `dir`, as the probe reports the sample format. +fn format_of(dir: &Path, name: &str) -> String { + format!("{:?}", soundcraft_audio_io::probe(&std::fs::read(dir.join(name)).unwrap(), None).unwrap().sample_format) +} + +/// Exit 1, nothing on stdout, and exactly `error: {message}` on stderr. +#[track_caller] +fn assert_fails(got: &Invoke, message: &str) { + assert_eq!(got.code, 1, "stderr: {}", got.stderr); + assert_eq!(got.stdout, ""); + assert_eq!(got.stderr, format!("error: {message}\n")); +} + +/// Exit 0 with nothing on either stream. +#[track_caller] +fn assert_silent_success(got: &Invoke) { + assert_eq!((got.code, got.stdout.as_str(), got.stderr.as_str()), (0, "", "")); +} + +/// Exit 2, nothing on stdout, and a clap usage error starting with `error: {message}` on stderr. +#[track_caller] +fn assert_usage_error(got: &Invoke, message: &str) { + assert_eq!(got.code, 2, "stderr: {}", got.stderr); + assert_eq!(got.stdout, ""); + assert!(got.stderr.starts_with(&format!("error: {message}")), "{}", got.stderr); +} + +/// The OS's message for a missing file: error 2 is `ENOENT` on Unix and `ERROR_FILE_NOT_FOUND` on Windows. +fn not_found() -> std::io::Error { + std::io::Error::from_raw_os_error(2) +} + +/// Exit 1 with the refused-connection message for `port`; the OS's text for the refusal differs by platform. +#[track_caller] +fn assert_refused(got: &Invoke, port: u16) { + assert_eq!((got.code, got.stdout.as_str()), (1, "")); + let prefix = format!("error: cannot reach SoundCraft at 127.0.0.1:{port}: "); + assert!(got.stderr.starts_with(&prefix) && got.stderr.contains(" (os error ") && got.stderr.ends_with(")\n"), "{}", got.stderr); +} + +// ---- unknown flags ---- + +#[test] +fn unknown_flags_stop_the_work_before_it_starts() { + let dir = sandbox("unknown-flag-reject"); + assert_usage_error(&invoke(&dir, &["convert", "in.wav", "out.wav", "--bogus"]), "unexpected argument '--bogus' found"); + assert_usage_error(&invoke(&dir, &["run", "--bogus", "--save", "s.scraft"]), "unexpected argument '--bogus' found"); + assert_usage_error(&invoke(&dir, &["script", "s.txt", "--bogus"]), "unexpected argument '--bogus' found"); + // A single-dash word is a flag too. + assert_usage_error(&invoke(&dir, &["convert", "in.wav", "out.wav", "-x"]), "unexpected argument '-x' found"); + assert!(!dir.join("out.wav").exists() && !dir.join("s.scraft").exists()); +} + +#[test] +fn convert_flags_may_come_before_or_between_the_positionals() { + let dir = sandbox("convert-order"); + let got = invoke(&dir, &["convert", "--rate", "44100", "in.wav", "a.wav"]); + assert_eq!((got.code, got.stdout.as_str(), got.stderr.as_str()), (0, "wrote a.wav (1 ch, 44100 Hz, 0.00 s)\n", "")); + let got = invoke(&dir, &["convert", "in.wav", "--rate", "44100", "b.wav"]); + assert_eq!((got.code, got.stdout.as_str(), got.stderr.as_str()), (0, "wrote b.wav (1 ch, 44100 Hz, 0.00 s)\n", "")); + assert_eq!((rate_of(&dir, "a.wav"), rate_of(&dir, "b.wav")), (44_100, 44_100)); +} + +// ---- value flags ---- + +#[test] +fn convert_value_flags_take_a_separate_or_inline_value() { + let dir = sandbox("convert-values"); + assert_eq!(invoke(&dir, &["convert", "in.wav", "c.wav", "--rate", "44100"]).stdout, "wrote c.wav (1 ch, 44100 Hz, 0.00 s)\n"); + assert_eq!(invoke(&dir, &["convert", "in.wav", "d.wav", "--rate=44100"]).stdout, "wrote d.wav (1 ch, 44100 Hz, 0.00 s)\n"); + assert_eq!(rate_of(&dir, "c.wav"), 44_100); + assert_eq!(rate_of(&dir, "d.wav"), 44_100); + assert_eq!(invoke(&dir, &["convert", "in.wav", "e.wav", "--bit-depth", "16"]).code, 0); + assert_eq!(invoke(&dir, &["convert", "in.wav", "f.wav", "--bit-depth=32f"]).code, 0); + assert_eq!(format_of(&dir, "e.wav"), "Int16"); + assert_eq!(format_of(&dir, "f.wav"), "Float32"); + assert_eq!(invoke(&dir, &["convert", "in.wav", "h.wav", "--bit-depth", "32"]).code, 0); + assert_eq!(format_of(&dir, "h.wav"), "Float32"); + assert_eq!(invoke(&dir, &["convert", "in.wav", "g.wav"]).code, 0); + assert_eq!(format_of(&dir, "g.wav"), "Int24"); +} + +#[test] +fn convert_value_flag_without_a_value_or_with_a_bad_one_fails() { + let dir = sandbox("convert-bad-values"); + assert_usage_error(&invoke(&dir, &["convert", "in.wav", "o.wav", "--rate"]), "a value is required for '--rate ' but none was supplied"); + assert_usage_error(&invoke(&dir, &["convert", "in.wav", "o.wav", "--bit-depth"]), "a value is required for '--bit-depth <16|24|32f>'"); + // A following flag is the next flag, not the value. + assert_usage_error(&invoke(&dir, &["convert", "in.wav", "o.wav", "--rate", "--normalize"]), "a value is required for '--rate '"); + assert_usage_error(&invoke(&dir, &["convert", "in.wav", "o.wav", "--normalize=1"]), "unexpected value '1' for '--normalize' found"); + assert!(!dir.join("o.wav").exists()); +} + +#[test] +fn repeated_flags_are_usage_errors_in_either_spelling() { + let dir = sandbox("repeat"); + for args in [ + &["convert", "in.wav", "a.wav", "--rate", "44100", "--rate", "22050"][..], + &["convert", "in.wav", "a.wav", "--rate=22050", "--rate", "44100"], + &["convert", "in.wav", "a.wav", "--bit-depth", "16", "--bit-depth", "24"], + &["convert", "in.wav", "a.wav", "--normalize", "--normalize"], + &["run", "--save", "a.scraft", "--save", "b.scraft"], + &["run", "--demo", "--demo"], + &["run", "--demo", "--sample"], + &["script", "s.txt", "--keep-going", "--keep-going"], + &["app", "--port", "1", "--port", "2", "x"], + &["mcp", "--demo", "--demo"], + ] { + let got = invoke(&dir, args); + assert_usage_error(&got, "the argument '--"); + assert_eq!(got.code, 2, "{args:?}"); + } + assert!(!dir.join("a.wav").exists() && !dir.join("a.scraft").exists()); +} + +#[test] +fn convert_input_and_output_errors() { + let dir = sandbox("convert-io"); + assert_fails(&invoke(&dir, &["convert", "nope.wav", "o.wav"]), &format!("nope.wav: {}", not_found())); + assert_fails(&invoke(&dir, &["convert", "in.wav", "o.xyz"]), "o.xyz: cannot write `.xyz` files (supported: wav, wave, bwf, aif, aiff, flac)"); +} + +#[test] +fn run_value_flags_take_a_separate_or_inline_value() { + let dir = sandbox("run-values"); + let tracks = "track.new: {\"tracks\":[1]}\n"; + assert_eq!(invoke(&dir, &["run", "--cmd", "track.new"]).stdout, tracks); + assert_eq!(invoke(&dir, &["run", "--cmd=track.new"]).stdout, tracks); + let save = invoke(&dir, &["run", "--save", "s1.scraft"]); + assert_eq!((save.code, save.stdout.as_str()), (0, "{\"audio_files_written\":0,\"path\":\"s1.scraft\"}\n")); + let save = invoke(&dir, &["run", "--save=s2.scraft"]); + assert_eq!((save.code, save.stdout.as_str()), (0, "{\"audio_files_written\":0,\"path\":\"s2.scraft\"}\n")); + assert!(dir.join("s1.scraft").exists() && dir.join("s2.scraft").exists()); + // --in in both spellings loads the saved session. + for args in [["run", "--in", "s1.scraft", "--inspect"], ["run", "--in=s1.scraft", "--inspect", ""]] { + let args: Vec<&str> = args.into_iter().filter(|a| !a.is_empty()).collect(); + let got = invoke(&dir, &args); + assert_eq!(got.code, 0, "{args:?}: {}", got.stderr); + assert!(got.stdout.contains("\"dirty\": false"), "{args:?}: {}", got.stdout); + } + // --bounce with --start/--end in both spellings. + let spaced = invoke(&dir, &["run", "--demo", "--bounce", "b1.wav", "--start", "0", "--end", "0.1"]); + assert_eq!(spaced.code, 0, "{}", spaced.stderr); + assert!(spaced.stdout.contains("\"path\":\"b1.wav\"") && spaced.stdout.contains("\"seconds\":0.1"), "{}", spaced.stdout); + let inline = invoke(&dir, &["run", "--demo", "--bounce=b2.wav", "--start=0", "--end=0.1"]); + assert_eq!(inline.code, 0, "{}", inline.stderr); + assert!(inline.stdout.contains("\"path\":\"b2.wav\"") && inline.stdout.contains("\"seconds\":0.1"), "{}", inline.stdout); + assert!(dir.join("b1.wav").exists() && dir.join("b2.wav").exists()); +} + +#[test] +fn run_cmd_repeats_and_runs_every_occurrence_in_order() { + let dir = sandbox("run-repeat"); + assert_eq!(invoke(&dir, &["run", "--cmd", "track.new", "--cmd=track.new"]).stdout, "track.new: {\"tracks\":[1]}\ntrack.new: {\"tracks\":[2]}\n"); +} + +#[test] +fn negative_times_are_values_but_other_dash_words_are_not() { + let dir = sandbox("negative-time"); + // The engine counts a position before 0, so the bounce starts a second ahead of the session. + for args in [&["--start", "-1"][..], &["--start=-1"]] { + let mut full = vec!["run", "--demo", "--end", "0.1", "--bounce", "b.wav"]; + full.extend(args); + let got = invoke(&dir, &full); + assert_eq!(got.code, 0, "{}", got.stderr); + assert!(got.stdout.contains("\"seconds\":1.1"), "{}", got.stdout); + } + assert_usage_error(&invoke(&dir, &["run", "--bounce", "-x.wav"]), "unexpected argument '-x' found"); + assert_usage_error(&invoke(&dir, &["run", "--cmd", "-x"]), "unexpected argument '-x' found"); +} + +#[test] +fn script_runs_lines_and_takes_options_before_or_after_the_file() { + let dir = sandbox("script"); + let got = invoke(&dir, &["script", "s.txt"]); + assert_eq!((got.code, got.stdout.as_str(), got.stderr.as_str()), (0, "track.new: {\"tracks\":[1]}\n", "")); + let got = invoke(&dir, &["script", "s.txt", "--save", "sv.scraft"]); + assert_eq!(got.stdout, "track.new: {\"tracks\":[1]}\n{\"audio_files_written\":0,\"path\":\"sv.scraft\"}\n"); + assert!(dir.join("sv.scraft").exists()); + assert_eq!(invoke(&dir, &["script", "s.txt", "--keep-going"]).stdout, "track.new: {\"tracks\":[1]}\n"); + assert_eq!(invoke(&dir, &["script", "--keep-going", "s.txt"]).stdout, "track.new: {\"tracks\":[1]}\n"); + assert_fails(&invoke(&dir, &["script", "nope.txt"]), &format!("nope.txt: {}", not_found())); + assert_fails(&invoke(&dir, &["script", "s.txt", "--in=nope.scraft"]), &format!("nope.scraft: {}", not_found())); +} + +#[test] +fn script_line_errors_stop_unless_keep_going() { + let dir = sandbox("script-errors"); + std::fs::write(dir.join("bad.txt"), "nosuch.cmd {}\ntrack.new {}\n").unwrap(); + std::fs::write(dir.join("badjson.txt"), "track.new {x\n").unwrap(); + // A stopping line error carries the `error:` prefix; a --keep-going one is a bare line. + let stop = invoke(&dir, &["script", "bad.txt"]); + assert_eq!((stop.code, stop.stdout.as_str()), (1, "")); + assert_eq!(stop.stderr, "error: bad.txt:1: nosuch.cmd: unknown command `nosuch.cmd`\n"); + let keep = invoke(&dir, &["script", "bad.txt", "--keep-going"]); + assert_eq!((keep.code, keep.stdout.as_str()), (0, "track.new: {\"tracks\":[1]}\n")); + assert_eq!(keep.stderr, "bad.txt:1: nosuch.cmd: unknown command `nosuch.cmd`\n"); + assert_fails(&invoke(&dir, &["script", "badjson.txt"]), "badjson.txt:1: bad JSON: key must be a string at line 1 column 2"); +} + +#[test] +fn info_prints_json_for_audio_and_fails_for_missing_files() { + let dir = sandbox("info"); + let got = invoke(&dir, &["info", "in.wav"]); + assert_eq!((got.code, got.stderr.as_str()), (0, "")); + assert!( + got.stdout.contains("\"channels\": 1") + && got.stdout.contains("\"sample_rate\": 48000") + && got.stdout.contains("\"sample_format\": \"Int16\""), + "{}", + got.stdout + ); + assert_fails(&invoke(&dir, &["info", "nope.wav"]), &format!("nope.wav: {}", not_found())); +} + +/// The extension decides a session in any case, as the desktop app's `open_path` does. +#[test] +fn info_reads_a_session_whatever_the_extension_case() { + let dir = sandbox("info-session-case"); + assert_eq!(invoke(&dir, &["run", "--demo", "--save", "S.SCRAFT"]).code, 0); + let got = invoke(&dir, &["info", "S.SCRAFT"]); + assert_eq!((got.code, got.stderr.as_str()), (0, "")); + assert!(got.stdout.starts_with("SESSION NAME:"), "{}", got.stdout); +} + +#[test] +fn info_compact_is_the_same_json_on_one_line() { + let dir = sandbox("info-compact"); + let (pretty, compact) = (invoke(&dir, &["info", "in.wav"]), invoke(&dir, &["info", "in.wav", "--compact"])); + assert_eq!((compact.code, compact.stderr.as_str()), (0, "")); + assert_eq!(compact.stdout.trim_end().lines().count(), 1, "{}", compact.stdout); + assert_eq!( + serde_json::from_str::(&pretty.stdout).unwrap(), + serde_json::from_str::(&compact.stdout).unwrap() + ); +} + +#[test] +fn parity_writes_a_report_with_write_in_both_spellings() { + let dir = sandbox("parity"); + let stdout_report = invoke(&dir, &["parity"]); + assert_eq!(stdout_report.code, 0); + assert!(stdout_report.stdout.starts_with("# Parity report\n"), "{}", stdout_report.stdout); + for (flag, name) in [(vec!["--write", "p1.md"], "p1.md"), (vec!["--write=p2.md"], "p2.md")] { + let mut args = vec!["parity"]; + args.extend(flag); + let got = invoke(&dir, &args); + assert_eq!(got.code, 0, "{}", got.stderr); + assert!(got.stdout.starts_with(&format!("wrote {name}: ")), "{}", got.stdout); + assert!(std::fs::read_to_string(dir.join(name)).unwrap().starts_with("# Parity report\n")); + } + assert_usage_error(&invoke(&dir, &["parity", "--write"]), "a value is required for '--write ' but none was supplied"); +} + +// ---- outputs that replace an input ---- + +/// `in.wav`, `s.txt` and a saved session `s.scraft`. +fn sandbox_with_session(tag: &str) -> PathBuf { + let dir = sandbox(tag); + assert_eq!(invoke(&dir, &["run", "--save", "s.scraft"]).code, 0); + dir +} + +#[test] +fn an_output_that_is_an_input_is_refused_before_any_work() { + let dir = sandbox_with_session("out-is-in"); + std::fs::hard_link(dir.join("in.wav"), dir.join("hard.wav")).unwrap(); + let (wav, session, script) = + (std::fs::read(dir.join("in.wav")).unwrap(), std::fs::read(dir.join("s.scraft")).unwrap(), std::fs::read(dir.join("s.txt")).unwrap()); + let in_wav = dir.join("in.wav"); + let absolute = in_wav.to_str().unwrap(); + for out in ["in.wav", "./in.wav", "../{}/in.wav", absolute, "hard.wav"] { + let out = out.replace("{}", dir.file_name().unwrap().to_str().unwrap()); + let got = invoke(&dir, &["convert", "in.wav", &out, "--rate", "44100"]); + assert_usage_error(&got, &format!(" {out} is the file, which it would replace: choose another file, or pass --in-place")); + } + // Nothing ran: no --cmd result, no bounce, no save. + let refused: &[(&[&str], &str)] = &[ + (&["run", "--in", "s.scraft", "--cmd", "track.new", "--save", "s.scraft"], "--save s.scraft is the --in session"), + (&["run", "--in=./s.scraft", "--cmd", "track.new", "--bounce", "s.scraft"], "--bounce s.scraft is the --in session"), + // --in-place lets only --save replace the session. + ( + &["run", "--in", "s.scraft", "--in-place", "--cmd", "track.new", "--bounce", "./s.scraft"], + "--bounce ./s.scraft is the --in session, which it would replace with audio: choose another file", + ), + (&["script", "s.txt", "--in", "s.scraft", "--in-place", "--bounce", "s.scraft"], "--bounce s.scraft is the --in session"), + (&["script", "s.txt", "--in", "s.scraft", "--save", "./s.scraft"], "--save ./s.scraft is the --in session"), + (&["script", "s.txt", "--save", "./s.txt"], "--save ./s.txt is the script file, which it would replace: choose another file"), + (&["script", "s.txt", "--in", "s.scraft", "--in-place", "--bounce", "s.txt"], "--bounce s.txt is the script file"), + (&["run", "--demo", "--cmd", "track.new", "--bounce", "b.wav", "--save", "./b.wav"], "--bounce and --save both name ./b.wav"), + ]; + for (args, message) in refused { + let got = invoke(&dir, args); + assert_usage_error(&got, message); + assert!(got.stderr.contains("\nUsage: soundcraft-cli "), "{}", got.stderr); + } + assert_eq!(std::fs::read(dir.join("in.wav")).unwrap(), wav); + assert_eq!(std::fs::read(dir.join("s.scraft")).unwrap(), session); + assert_eq!(std::fs::read(dir.join("s.txt")).unwrap(), script); + assert!(!dir.join("b.wav").exists()); + assert_usage_error( + &invoke(&dir, &["run", "--in-place", "--save", "s.scraft"]), + "the following required arguments were not provided:\n --in ", + ); +} + +#[cfg(unix)] +#[test] +fn an_output_linked_to_the_input_is_refused() { + let dir = sandbox_with_session("out-is-in-link"); + std::os::unix::fs::symlink("in.wav", dir.join("link.wav")).unwrap(); + std::os::unix::fs::symlink(".", dir.join("here")).unwrap(); + assert_usage_error(&invoke(&dir, &["convert", "in.wav", "link.wav"]), " link.wav is the file"); + assert_usage_error(&invoke(&dir, &["run", "--in", "s.scraft", "--save", "here/s.scraft"]), "--save here/s.scraft is the --in session"); + // Two outputs that don't exist yet, one through a linked folder. + assert_usage_error(&invoke(&dir, &["run", "--bounce", "here/b.wav", "--save", "b.wav"]), "--bounce and --save both name b.wav"); + assert_eq!(rate_of(&dir, "in.wav"), 48_000); +} + +#[test] +fn in_place_replaces_the_input_and_other_files_are_still_replaced() { + let dir = sandbox_with_session("in-place"); + let got = invoke(&dir, &["convert", "in.wav", "./in.wav", "--rate", "44100", "--in-place"]); + assert_eq!(got.code, 0, "{}", got.stderr); + assert_eq!(rate_of(&dir, "in.wav"), 44_100); + let got = invoke(&dir, &["run", "--in", "s.scraft", "--cmd", "track.new", "--save", "s.scraft", "--in-place"]); + assert_eq!(got.stdout, "track.new: {\"tracks\":[1]}\n{\"audio_files_written\":0,\"path\":\"s.scraft\"}\n", "{}", got.stderr); + // The saved session holds that track, so the next one is the second. + assert_eq!(invoke(&dir, &["run", "--in", "s.scraft", "--cmd", "track.new"]).stdout, "track.new: {\"tracks\":[2]}\n"); + // An existing file that is no input is replaced without --in-place. + for rate in ["32000", "22050"] { + let got = invoke(&dir, &["convert", "in.wav", "o.wav", "--rate", rate]); + assert_eq!(got.code, 0, "{}", got.stderr); + } + assert_eq!(rate_of(&dir, "o.wav"), 22_050); + // other.scraft gets the script's track, then is replaced by s.scraft as it is. + let next_track = |session: &str| invoke(&dir, &["run", "--in", session, "--cmd", "track.new"]).stdout; + let got = invoke(&dir, &["script", "s.txt", "--in", "s.scraft", "--save", "other.scraft"]); + assert_eq!(got.code, 0, "{}", got.stderr); + assert_eq!(next_track("other.scraft"), "track.new: {\"tracks\":[3]}\n"); + let got = invoke(&dir, &["run", "--in", "s.scraft", "--save", "other.scraft"]); + assert_eq!(got.code, 0, "{}", got.stderr); + assert_eq!(next_track("other.scraft"), "track.new: {\"tracks\":[2]}\n"); +} + +/// A write that fails leaves the input as it was: the new bytes go to a new file first. +#[cfg(unix)] +#[test] +fn failed_in_place_convert_keeps_the_input() { + use std::os::unix::fs::PermissionsExt; + let dir = sandbox("in-place-fails"); + let before = std::fs::read(dir.join("in.wav")).unwrap(); + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o555)).unwrap(); + let can_create = std::fs::File::create(dir.join("probe")).is_ok(); + if !can_create { + let got = invoke(&dir, &["convert", "in.wav", "in.wav", "--rate", "44100", "--in-place"]); + assert_eq!(got.code, 1, "{}", got.stderr); + assert_eq!(std::fs::read(dir.join("in.wav")).unwrap(), before); + } + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)).unwrap(); + assert!(std::fs::read_dir(&dir).unwrap().all(|e| !e.unwrap().file_name().to_string_lossy().ends_with(".part"))); +} + +/// An in-place output that is a link to the input replaces the input, and the link stays a link. +#[cfg(unix)] +#[test] +fn in_place_through_a_link_replaces_the_linked_input() { + let dir = sandbox_with_session("in-place-link"); + std::os::unix::fs::symlink("in.wav", dir.join("link.wav")).unwrap(); + std::os::unix::fs::symlink("s.scraft", dir.join("link.scraft")).unwrap(); + let got = invoke(&dir, &["convert", "in.wav", "link.wav", "--rate", "44100", "--in-place"]); + assert_eq!(got.stdout, "wrote link.wav (1 ch, 44100 Hz, 0.00 s)\n", "{}", got.stderr); + assert_eq!(rate_of(&dir, "in.wav"), 44_100); + let got = invoke(&dir, &["run", "--in", "s.scraft", "--cmd", "track.new", "--save", "link.scraft", "--in-place"]); + assert_eq!(got.code, 0, "{}", got.stderr); + // s.scraft holds that track, so the next one is the second. + assert_eq!(invoke(&dir, &["run", "--in", "s.scraft", "--cmd", "track.new"]).stdout, "track.new: {\"tracks\":[2]}\n"); + for link in ["link.wav", "link.scraft"] { + assert!(dir.join(link).symlink_metadata().unwrap().file_type().is_symlink(), "{link}"); + } +} + +// ---- app ---- + +#[test] +fn app_port_comes_from_the_flag_then_the_environment() { + let dir = sandbox("app-env"); + assert_refused(&invoke_env(&dir, &["app", "session.inspect"], Some("1")), 1); + assert_refused(&invoke_env(&dir, &["app", "--port", "2", "session.inspect"], Some("1")), 2); + // The flag wins before the variable is read, so a bad one doesn't matter. + assert_refused(&invoke_env(&dir, &["app", "--port", "2", "session.inspect"], Some("abc")), 2); +} + +#[test] +fn app_takes_an_empty_port_variable_as_unset() { + if std::net::TcpStream::connect("127.0.0.1:7979").is_ok() { + eprintln!("skipped: something listens on 127.0.0.1:7979, the default port"); + return; + } + let dir = sandbox("app-env-empty"); + for empty in ["", " "] { + assert_refused(&invoke_env(&dir, &["app", "session.inspect"], Some(empty)), 7979); + } +} + +#[test] +fn a_bad_port_variable_is_a_usage_error_naming_it() { + let dir = sandbox("app-env-bad"); + for bad in ["abc", "70000", ":80"] { + assert_usage_error( + &invoke_env(&dir, &["app", "session.inspect"], Some(bad)), + &format!("invalid value '{bad}' for 'SOUNDCRAFT_CONTROL_PORT': expected a port, 0-65535, or host:port"), + ); + } +} + +#[test] +fn app_help_names_the_port_variable() { + let got = invoke(&sandbox("app-help"), &["app", "--help"]); + assert_eq!(got.code, 0, "{}", got.stderr); + assert!(got.stdout.contains("[env: SOUNDCRAFT_CONTROL_PORT=]\n [default: 7979]\n"), "{}", got.stdout); +} + +// ---- mcp ---- + +#[test] +fn mcp_accepts_its_flags_in_both_spellings_and_exits_cleanly_on_empty_stdin() { + let dir = sandbox("mcp-ok"); + for args in [vec!["mcp"], vec!["mcp", "--demo"], vec!["mcp", "--connect", "1"], vec!["mcp", "--connect=1"]] { + assert_silent_success(&invoke(&dir, &args)); + } + // A root prints the confinement line to stderr (never stdout). + for args in [&["mcp", "--automation-read-root", "."][..], &["mcp", "--automation-read-root=."][..]] { + let got = invoke(&dir, args); + assert_eq!((got.code, got.stdout.as_str()), (0, "")); + assert!(got.stderr.starts_with("soundcraft-cli: files confined: read ") && got.stderr.ends_with("; write none\n"), "{}", got.stderr); + } + // A repeated write-root flag is accepted in either spelling. + let twice = invoke(&dir, &["mcp", "--automation-write-root=.", "--automation-write-root", "."]); + assert_eq!(twice.code, 0); + assert!( + twice.stderr.starts_with("soundcraft-cli: files confined: read none; write ") && !twice.stderr.contains("; write none"), + "{}", + twice.stderr + ); +} + +#[test] +fn mcp_demo_with_connect_is_a_usage_error() { + let dir = sandbox("mcp-demo-connect"); + assert_usage_error(&invoke(&dir, &["mcp", "--demo", "--connect", "1"]), "the argument '--demo' cannot be used with '--connect '"); +} + +#[test] +fn mcp_roots_with_connect_are_a_usage_error() { + let dir = sandbox("mcp-roots-connect"); + // Refused before any folder is opened, so a missing one isn't reported. + assert_usage_error( + &invoke(&dir, &["mcp", "--connect", "1", "--automation-write-root", "missing"]), + "the argument '--connect ' cannot be used with '--automation-write-root '", + ); + assert_usage_error( + &invoke(&dir, &["mcp", "--automation-read-root=.", "--connect=1"]), + "the argument '--automation-read-root ' cannot be used with '--connect '", + ); +} + +/// A name that isn't valid Unicode works wherever the tool itself reads or writes the file +/// (`convert`); a session or bounce the engine opens or writes by text name is refused (exit 1), +/// naming the file. Run in-process: the names need no shell. +#[cfg(unix)] +#[test] +fn non_unicode_file_names() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + let dir = sandbox("nonunicode"); + let latin1 = |ext: &str| { + let mut n = b"caf\xe9.".to_vec(); + n.extend_from_slice(ext.as_bytes()); + dir.join(OsString::from_vec(n)) + }; + let run = |args: Vec| { + let (mut out, mut err) = (Vec::new(), Vec::new()); + let code = soundcraft_cli::run(args, &mut out, &mut err); + (code, String::from_utf8_lossy(&err).into_owned()) + }; + let (code, err) = run(vec!["convert".into(), dir.join("in.wav").into(), latin1("flac").into()]); + assert_eq!(code, 0, "{err}"); + assert!(std::fs::metadata(latin1("flac")).unwrap().len() > 0); + // A session opens by path; only paths that go into the engine's JSON must be Unicode. + let (code, err) = run(vec!["run".into(), "--demo".into(), "--save".into(), dir.join("s.scraft").into()]); + assert_eq!(code, 0, "{err}"); + std::fs::rename(dir.join("s.scraft"), latin1("scraft")).unwrap(); + let (code, err) = run(vec!["info".into(), latin1("scraft").into()]); + assert_eq!(code, 0, "{err}"); + let (code, err) = run(vec!["run".into(), "--demo".into(), "--bounce".into(), latin1("wav").into()]); + assert_eq!(code, 1); + assert!(err.contains("caf\u{FFFD}.") && err.contains("not valid Unicode"), "{err}"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/apps/soundcraft-cli/tests/it/cli.rs b/apps/soundcraft-cli/tests/it/cli.rs index 82feb1ce..2a95cb38 100644 --- a/apps/soundcraft-cli/tests/it/cli.rs +++ b/apps/soundcraft-cli/tests/it/cli.rs @@ -209,13 +209,13 @@ fn mcp_automation_root_flags_are_checked_up_front() { fails(&["--automation-write-root", dir, "--connect", "7979"], "--connect"); // #874: a flag may be given again, but every folder it names must exist. fails(&["--automation-read-root", dir, "--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root"); - fails(&["--automation-write-root", dir, "--automation-write-root"], "needs a folder"); - fails(&["--automation-read-root"], "needs a folder"); - fails(&["--automation-write-root", "--demo"], "needs a folder"); + fails(&["--automation-write-root", dir, "--automation-write-root"], "a value is required"); + fails(&["--automation-read-root"], "a value is required"); + fails(&["--automation-write-root", "--demo"], "a value is required"); fails(&["--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root"); - fails(&["--automation-write-root="], "needs a non-empty folder"); + fails(&["--automation-write-root="], "a value is required"); // A misspelled root flag must not start an unconfined server. - fails(&["--automation-read-roots", dir], "unknown mcp option"); + fails(&["--automation-read-roots", dir], "unexpected argument '--automation-read-roots'"); // A file is not a folder. std::fs::write(base.join("f.txt"), "x").unwrap(); fails(&["--automation-write-root", base.join("f.txt").to_str().unwrap()], "cannot open the automation write root"); @@ -224,9 +224,9 @@ fn mcp_automation_root_flags_are_checked_up_front() { #[test] fn usage_lists_the_root_flags() { - let o = run(&["--help"]); + let o = run(&["mcp", "--help"]); let help = String::from_utf8_lossy(&o.stdout).into_owned(); - assert!(help.contains("--automation-read-root DIR") && help.contains("--automation-write-root DIR"), "{help}"); + assert!(help.contains("--automation-read-root ") && help.contains("--automation-write-root "), "{help}"); } /// Without `--start`/`--end`, `--bounce` covers the whole session, not a stray edit selection diff --git a/apps/soundcraft-cli/tests/it/main.rs b/apps/soundcraft-cli/tests/it/main.rs index c573712b..f1c44788 100644 --- a/apps/soundcraft-cli/tests/it/main.rs +++ b/apps/soundcraft-cli/tests/it/main.rs @@ -1,5 +1,11 @@ //! Integration tests, built as one test binary: each file is a module. +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod behaviour; #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] mod cli; mod closed_stdout; +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod man_and_completions; +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod run; diff --git a/apps/soundcraft-cli/tests/it/man_and_completions.rs b/apps/soundcraft-cli/tests/it/man_and_completions.rs new file mode 100644 index 00000000..8ed422f0 --- /dev/null +++ b/apps/soundcraft-cli/tests/it/man_and_completions.rs @@ -0,0 +1,55 @@ +//! The clap definition is consistent, and every shell's completions and every man page +//! (`examples/man-and-completions.rs`) can be generated from it. + +use clap::ValueEnum; +use clap_complete::Shell; + +// The example itself, so the test writes the files exactly as the packages get them. +#[allow(dead_code)] +#[path = "../../examples/man-and-completions.rs"] +mod example; + +#[test] +fn command_is_consistent() { + soundcraft_cli::command().debug_assert(); +} + +#[test] +fn completions_generate_for_every_shell() { + for &shell in Shell::value_variants() { + let mut out = Vec::new(); + clap_complete::generate(shell, &mut soundcraft_cli::command(), "soundcraft-cli", &mut out); + assert!(!out.is_empty(), "{shell}"); + } +} + +#[test] +fn man_pages_render_for_the_command_and_every_subcommand() { + let cmd = soundcraft_cli::command(); + for page in std::iter::once(&cmd).chain(cmd.get_subcommands()) { + let mut out = Vec::new(); + clap_mangen::Man::new(page.clone()).render(&mut out).unwrap(); + assert!(!out.is_empty(), "{}", page.get_name()); + } +} + +/// The example writes the five completion files under the names `packaging/linux/package.sh` and +/// `packaging/freebsd/package.sh` install, `soundcraft-cli.1` and a page per subcommand, and no +/// pages for the `help` subcommand. +#[test] +fn the_example_writes_the_files_the_packages_install() { + let dir = std::env::temp_dir().join(format!("sc-cli-{}-man-and-completions", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + example::generate(&dir).unwrap(); + + let mut written: Vec = std::fs::read_dir(&dir).unwrap().map(|e| e.unwrap().file_name().into_string().unwrap()).collect(); + written.sort(); + let completions = ["soundcraft-cli.bash", "_soundcraft-cli", "soundcraft-cli.fish", "soundcraft-cli.elv", "_soundcraft-cli.ps1"]; + let subcommands = ["info", "convert", "run", "script", "commands", "describe", "app", "mcp", "parity", "plugins"]; + let mut expected: Vec = completions.map(String::from).into(); + expected.push("soundcraft-cli.1".into()); + expected.extend(subcommands.iter().map(|s| format!("soundcraft-cli-{s}.1"))); + expected.sort(); + assert_eq!(written, expected); + std::fs::remove_dir_all(&dir).unwrap(); +} diff --git a/apps/soundcraft-cli/tests/it/run.rs b/apps/soundcraft-cli/tests/it/run.rs new file mode 100644 index 00000000..60ffff70 --- /dev/null +++ b/apps/soundcraft-cli/tests/it/run.rs @@ -0,0 +1,426 @@ +//! The CLI run in-process through `soundcraft_cli::run`: the tests that need no files, working +//! directory, environment or standard input. Those that do run the real binary (`behaviour.rs`). +//! A change to the command line's behaviour updates them on purpose. + +use soundcraft_cli::run; + +/// What [`run`] returned and wrote. +struct Invoke { + code: i32, + stdout: String, + stderr: String, +} + +fn invoke(args: &[&str]) -> Invoke { + let (mut stdout, mut stderr) = (Vec::new(), Vec::new()); + let code = run(args.iter().copied(), &mut stdout, &mut stderr); + Invoke { code, stdout: String::from_utf8(stdout).unwrap(), stderr: String::from_utf8(stderr).unwrap() } +} + +/// Exit 1, nothing on stdout, and exactly `error: {message}` on stderr. +#[track_caller] +fn assert_fails(got: &Invoke, message: &str) { + assert_eq!(got.code, 1, "stderr: {}", got.stderr); + assert_eq!(got.stdout, ""); + assert_eq!(got.stderr, format!("error: {message}\n")); +} + +/// Exit 0 with nothing on either stream. +#[track_caller] +fn assert_silent_success(got: &Invoke) { + assert_eq!((got.code, got.stdout.as_str(), got.stderr.as_str()), (0, "", "")); +} + +/// Exit 2, nothing on stdout, and a clap usage error on stderr: `error: ` with `message`, ending +/// with the `--help` hint. +#[track_caller] +fn assert_usage_error(got: &Invoke, message: &str) { + assert_eq!(got.code, 2, "stderr: {}", got.stderr); + assert_eq!(got.stdout, ""); + assert!(got.stderr.starts_with(&format!("error: {message}")), "{}", got.stderr); + assert!(got.stderr.ends_with("\n\nFor more information, try '--help'.\n"), "{}", got.stderr); +} + +/// Exit 0, help on stdout, nothing on stderr. +#[track_caller] +fn assert_help(got: &Invoke) -> &str { + assert_eq!((got.code, got.stderr.as_str()), (0, ""), "{}", got.stdout); + &got.stdout +} + +/// The OS's message for a missing file: error 2 is `ENOENT` on Unix and `ERROR_FILE_NOT_FOUND` on Windows. +fn not_found() -> std::io::Error { + std::io::Error::from_raw_os_error(2) +} + +/// Exit 1 with the refused-connection message for `port`; the OS's text for the refusal differs by platform. +#[track_caller] +fn assert_refused(got: &Invoke, port: u16) { + assert_eq!((got.code, got.stdout.as_str()), (1, "")); + let prefix = format!("error: cannot reach SoundCraft at 127.0.0.1:{port}: "); + assert!(got.stderr.starts_with(&prefix) && got.stderr.contains(" (os error ") && got.stderr.ends_with(")\n"), "{}", got.stderr); +} + +#[test] +fn top_level_help_lists_every_subcommand_on_stdout() { + let long = invoke(&["--help"]); + let help = assert_help(&long); + assert!(help.starts_with("soundcraft-cli: SoundCraft without a window\n\nUsage: soundcraft-cli \n"), "{help}"); + for sub in ["info", "convert", "run", "script", "commands", "describe", "app", "mcp", "parity", "plugins", "help"] { + assert!(help.contains(&format!("\n {sub} ")), "{sub}: {help}"); + } + assert!(help.contains("-V, --version") && !help.contains("\n version "), "{help}"); + for spelling in ["-h", "help"] { + let got = invoke(&[spelling]); + assert_eq!((got.code, got.stdout, got.stderr), (0, long.stdout.clone(), String::new()), "{spelling}"); + } +} + +#[test] +fn version_flags_print_the_long_version_to_stdout() { + let expected = format!("soundcraft-cli {} ({} {})\n", env!("CARGO_PKG_VERSION"), std::env::consts::OS, std::env::consts::ARCH); + for spelling in ["--version", "-V"] { + let got = invoke(&[spelling]); + assert_eq!((got.code, got.stdout, got.stderr), (0, expected.clone(), String::new()), "{spelling}"); + } +} + +#[test] +fn version_is_not_a_subcommand() { + assert_usage_error(&invoke(&["version"]), "unrecognized subcommand 'version'"); +} + +#[test] +fn help_subcommand_shows_that_subcommands_help() { + let got = invoke(&["help", "convert"]); + let help = assert_help(&got); + assert!(help.starts_with("Convert an audio file"), "{help}"); + assert_eq!(help, assert_help(&invoke(&["convert", "--help"]))); +} + +#[test] +fn help_names_every_format_the_command_reads_and_writes() { + use soundcraft_audio_io::FileFormat; + let read = format!("({})", FileFormat::readable_extensions().join(", ")); + let write = format!("its extension picks the format ({})", FileFormat::writable_extensions().join(", ")); + let convert = invoke(&["convert", "--help"]); + let convert = assert_help(&convert); + assert!(convert.contains(&read) && convert.contains(&write), "{convert}"); + let info = invoke(&["info", "--help"]); + assert!(assert_help(&info).contains(&read), "{}", info.stdout); +} + +#[test] +fn no_arguments_prints_the_help_to_stderr_and_exits_2() { + let got = invoke(&[]); + assert_eq!((got.code, got.stdout.as_str()), (2, "")); + assert_eq!(got.stderr, invoke(&["--help"]).stdout); +} + +#[test] +fn unknown_subcommand_is_a_usage_error() { + assert_usage_error(&invoke(&["bogus"]), "unrecognized subcommand 'bogus'"); + assert_usage_error(&invoke(&["bogus", "--help"]), "unrecognized subcommand 'bogus'"); + // Subcommand names are case sensitive and not abbreviated. + assert_usage_error(&invoke(&["INFO"]), "unrecognized subcommand 'INFO'"); + assert!(invoke(&["plug"]).stderr.contains("tip: a similar subcommand exists: 'plugins'")); + assert_usage_error(&invoke(&["--bogus"]), "unexpected argument '--bogus' found"); +} + +/// Each subcommand's help: its usage line, then every argument and flag it takes. +#[test] +fn every_subcommand_has_help_listing_its_arguments() { + let engine = [ + "--in ", + "--demo", + "--sample", + "--bounce ", + "--start ", + "--end ", + "--save ", + "--inspect", + "--compact", + "--in-place", + ]; + let cases: &[(&str, &str, &[&str])] = &[ + ("info", "info [OPTIONS] ", &["", "--compact"]), + ( + "convert", + "convert [OPTIONS] ", + &["", "", "--bit-depth <16|24|32f>", "--rate ", "--normalize", "--dither", "--no-dither", "--in-place"], + ), + ("run", "run [OPTIONS]", &["--cmd "]), + ("script", "script [OPTIONS] ", &["", "--keep-going"]), + ("commands", "commands [FILTER]", &["[FILTER]"]), + ("describe", "describe [OPTIONS] ", &["", "--compact"]), + ( + "app", + "app [OPTIONS] [JSON]", + &["--port

", "--compact", "", "[JSON]", "[env: SOUNDCRAFT_CONTROL_PORT", "[default: 7979]"], + ), + ( + "mcp", + "mcp [OPTIONS]", + &["--connect ", "--demo", "--automation-read-root

", "--automation-write-root ", "conflict with --connect"], + ), + ("parity", "parity [OPTIONS]", &["--write "]), + ("plugins", "plugins", &[]), + ]; + for &(sub, usage, args) in cases { + let long = invoke(&[sub, "--help"]); + let help = assert_help(&long); + assert!(help.contains(&format!("\nUsage: soundcraft-cli {usage}\n")), "{sub}: {help}"); + let shared: &[&str] = if matches!(sub, "run" | "script") { &engine } else { &[] }; + for arg in args.iter().chain(shared).chain(&["-h, --help"]) { + assert!(help.contains(arg), "{sub} lacks {arg}: {help}"); + } + // `help SUB` is `SUB --help`; `-h` is the one-line-per-flag summary. + assert_eq!(invoke(&["help", sub]).stdout, long.stdout, "help {sub}"); + assert!(assert_help(&invoke(&[sub, "-h"])).contains(&format!("\nUsage: soundcraft-cli {usage}\n")), "{sub} -h"); + } +} + +#[test] +fn help_wins_over_a_mistake_before_it() { + let help = invoke(&["run", "--help"]).stdout; + assert_eq!(assert_help(&invoke(&["run", "--bounce", "--help"])), help); + assert_eq!(assert_help(&invoke(&["run", "--bogus", "--help"])), help); + assert!(assert_help(&invoke(&["convert", "--rate", "-h"])).contains("Usage: soundcraft-cli convert")); + assert!(assert_help(&invoke(&["mcp", "--connect", "--help"])).contains("Usage: soundcraft-cli mcp")); + // After `--`, `--help` is a value. + assert_eq!(invoke(&["info", "--", "--help"]).code, 1); +} + +#[test] +fn run_and_script_value_flags_without_a_value_are_usage_errors() { + for (flag, name) in [("--cmd", "ID[=JSON]"), ("--in", "S.scraft"), ("--save", "S.scraft"), ("--bounce", "OUT"), ("--start", "T"), ("--end", "T")] + { + let message = format!("a value is required for '{flag} <{name}>' but none was supplied"); + assert_usage_error(&invoke(&["run", flag]), &message); + if flag != "--cmd" { + assert_usage_error(&invoke(&["script", "s.txt", flag]), &message); + } + } + // A following flag is not the value. + assert_usage_error(&invoke(&["run", "--bounce", "--inspect"]), "a value is required for '--bounce '"); + assert_usage_error(&invoke(&["script", "s.txt", "--demo", "--bounce"]), "a value is required for '--bounce '"); +} + +#[test] +fn boolean_flags_reject_an_inline_value() { + assert_usage_error(&invoke(&["run", "--demo=1"]), "unexpected value '1' for '--demo' found; no more were expected"); + assert_usage_error(&invoke(&["run", "--inspect=1"]), "unexpected value '1' for '--inspect' found; no more were expected"); + assert_usage_error(&invoke(&["script", "s.txt", "--keep-going=1"]), "unexpected value '1' for '--keep-going' found; no more were expected"); + assert_usage_error(&invoke(&["mcp", "--demo=1"]), "unexpected value '1' for '--demo' found; no more were expected"); +} + +#[test] +fn missing_and_extra_positionals_are_usage_errors() { + assert_usage_error(&invoke(&["info"]), "the following required arguments were not provided:\n "); + assert_usage_error(&invoke(&["convert", "in.wav"]), "the following required arguments were not provided:\n "); + assert_usage_error(&invoke(&["script"]), "the following required arguments were not provided:\n "); + assert_usage_error(&invoke(&["describe"]), "the following required arguments were not provided:\n "); + assert_usage_error(&invoke(&["app", "--port", "1"]), "the following required arguments were not provided:\n "); + for args in [ + &["info", "a", "b"][..], + &["convert", "in.wav", "out.wav", "b"], + &["script", "s.txt", "b"], + &["run", "b"], + &["commands", "bounce", "b"], + &["describe", "file.bounce_mix", "b"], + &["app", "--port", "1", "session.inspect", "{}", "b"], + &["mcp", "b"], + &["parity", "b"], + &["plugins", "b"], + ] { + assert_usage_error(&invoke(args), "unexpected argument 'b' found"); + } +} + +#[test] +fn unknown_flags_are_usage_errors_on_every_subcommand() { + for args in [ + &["info", "in.wav", "--bogus"][..], + &["convert", "in.wav", "out.wav", "--bogus"], + &["run", "--bogus"], + &["script", "s.txt", "--bogus"], + &["commands", "--bogus"], + &["describe", "--bogus"], + &["app", "--bogus", "session.inspect"], + &["mcp", "--bogus"], + &["parity", "--bogus"], + &["plugins", "--bogus"], + ] { + assert_usage_error(&invoke(args), "unexpected argument '--bogus' found"); + } + // A single-dash word is a short flag, and there are none but -h. + assert_usage_error(&invoke(&["run", "-x"]), "unexpected argument '-x' found"); + assert_usage_error(&invoke(&["mcp", "--automation-bogus=x"]), "unexpected argument '--automation-bogus' found"); +} + +/// `args_os`: text that isn't valid Unicode reaches clap instead of aborting the process. +#[cfg(unix)] +#[test] +fn a_non_unicode_argument_is_a_usage_error_not_a_panic() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + let (mut stdout, mut stderr) = (Vec::new(), Vec::new()); + let code = run([OsString::from("describe"), OsString::from_vec(b"\xe9.wav".to_vec())], &mut stdout, &mut stderr); + assert_eq!((code, stdout.as_slice()), (2, &b""[..])); + assert!(String::from_utf8_lossy(&stderr).starts_with("error: invalid UTF-8 was detected in one or more arguments")); +} + +/// A bounce or save path goes to the engine as JSON text: a non-Unicode one fails before any command runs. +#[cfg(unix)] +#[test] +fn a_non_unicode_output_fails_before_the_commands_run() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + for flag in ["--bounce", "--save"] { + let (mut stdout, mut stderr) = (Vec::new(), Vec::new()); + let args = ["run", "--cmd", "track.new", flag].map(OsString::from); + let code = run(args.into_iter().chain([OsString::from_vec(b"\xe9.out".to_vec())]), &mut stdout, &mut stderr); + assert_eq!((code, stdout.as_slice()), (1, &b""[..]), "{flag}: {}", String::from_utf8_lossy(&stderr)); + } +} + +#[test] +fn run_demo_and_sample_are_the_same_flag() { + let demo = invoke(&["run", "--demo", "--inspect"]); + let sample = invoke(&["run", "--sample", "--inspect"]); + assert_eq!((demo.code, sample.code), (0, 0)); + assert_eq!(demo.stdout, sample.stdout); + assert!(demo.stdout.contains("\"name\": \"Verb\""), "{}", demo.stdout); +} + +/// `--compact` prints the same JSON on one line. +#[track_caller] +fn assert_compact_matches_pretty(pretty: &[&str], compact: &[&str]) { + let (p, c) = (invoke(pretty), invoke(compact)); + assert_eq!((p.code, c.code), (0, 0), "{} {}", p.stderr, c.stderr); + assert!(p.stdout.trim_end().contains('\n'), "pretty form spans lines: {}", p.stdout); + assert_eq!(c.stdout.trim_end().lines().count(), 1, "{}", c.stdout); + assert!(c.stdout.ends_with('\n')); + let (p, c): (serde_json::Value, serde_json::Value) = (serde_json::from_str(&p.stdout).unwrap(), serde_json::from_str(&c.stdout).unwrap()); + assert_eq!(p, c); +} + +#[test] +fn compact_prints_describe_and_run_inspect_on_one_line() { + assert_compact_matches_pretty(&["describe", "file.bounce_mix"], &["describe", "file.bounce_mix", "--compact"]); + assert_compact_matches_pretty(&["run", "--demo", "--inspect"], &["run", "--demo", "--inspect", "--compact"]); +} + +#[test] +fn compact_without_inspect_is_a_usage_error() { + assert_usage_error(&invoke(&["run", "--compact"]), "the following required arguments were not provided:\n --inspect"); +} + +#[test] +fn run_command_failures() { + assert_fails(&invoke(&["run", "--cmd", "nosuch.cmd"]), "nosuch.cmd: unknown command `nosuch.cmd`"); + assert_fails(&invoke(&["run", "--in", "nope.scraft"]), &format!("nope.scraft: {}", not_found())); + assert_fails(&invoke(&["run", "--in=nope.scraft"]), &format!("nope.scraft: {}", not_found())); +} + +/// Relations and typed values are checked by clap: exit 2, the message names the flag, and nothing runs. +#[test] +fn flag_relations_and_typed_values_are_usage_errors() { + for (args, message) in [ + (&["convert", "in.wav", "o.wav", "--dither", "--no-dither"][..], "the argument '--dither' cannot be used with '--no-dither'"), + (&["convert", "in.wav", "o.wav", "--rate", "0"], "invalid value '0' for '--rate '"), + (&["convert", "in.wav", "o.wav", "--rate=abc"], "invalid value 'abc' for '--rate '"), + (&["convert", "in.wav", "o.wav", "--bit-depth", "8"], "invalid value '8' for '--bit-depth <16|24|32f>'"), + (&["run", "--in", "s.scraft", "--demo"], "the argument '--in ' cannot be used with '--demo'"), + (&["script", "s.txt", "--in", "s.scraft", "--sample"], "the argument '--in ' cannot be used with '--demo'"), + (&["run", "--start", "1"], "the following required arguments were not provided:\n --bounce "), + (&["script", "s.txt", "--end", "1"], "the following required arguments were not provided:\n --bounce "), + (&["run", "--bounce", "b.wav", "--start", ""], "invalid value '' for '--start '"), + (&["run", "--bounce", "b.wav", "--start", "nan"], "invalid value 'nan' for '--start ': expected a finite number"), + (&["run", "--bounce", "b.wav", "--end", "inf"], "invalid value 'inf' for '--end ': expected a finite number"), + (&["run", "--bounce", "b.wav", "--end", "1e999"], "invalid value '1e999' for '--end ': expected a finite number"), + (&["run", "--cmd", "track.new={", "--bounce", "b.wav"], "invalid value 'track.new={' for '--cmd ': bad JSON:"), + (&["run", "--cmd", "=1"], "invalid value '=1' for '--cmd '"), + (&["describe", ""], "a value is required for ''"), + (&["commands", ""], "a value is required for '[FILTER]'"), + (&["app", ""], "a value is required for ''"), + ] { + let got = invoke(args); + assert_usage_error(&got, message); + } +} + +#[test] +fn a_bad_later_cmd_runs_none_of_them() { + assert_usage_error(&invoke(&["run", "--cmd", "track.new", "--cmd", "track.new={"]), "invalid value 'track.new={'"); +} + +#[test] +fn commands_filters_by_id_or_label() { + let all = invoke(&["commands"]); + assert_eq!((all.code, all.stderr.as_str()), (0, "")); + assert!(all.stdout.starts_with("session.new "), "{}", all.stdout); + let bounce = invoke(&["commands", "bounce"]); + assert!(bounce.stdout.contains("file.bounce_mix ") && !bounce.stdout.contains("session.new"), "{}", bounce.stdout); + // Matching is case-insensitive on the label only: an upper-case filter finds the labels, not the ids. + assert!(invoke(&["commands", "BOUNCE"]).stdout.contains("Bounce Mix...")); + assert_silent_success(&invoke(&["commands", "zzzzqq"])); +} + +#[test] +fn describe_prints_json_for_a_known_id_and_fails_for_an_unknown_one() { + let got = invoke(&["describe", "file.bounce_mix"]); + assert_eq!((got.code, got.stderr.as_str()), (0, "")); + assert!(got.stdout.starts_with("{\n \"id\": \"file.bounce_mix\",\n \"label\": \"Bounce Mix...\""), "{}", got.stdout); + assert_fails(&invoke(&["describe", "nosuch"]), "unknown command `nosuch`"); +} + +#[test] +fn plugins_lists_one_line_per_plugin() { + let got = invoke(&["plugins"]); + assert_eq!((got.code, got.stderr.as_str()), (0, "")); + assert!(got.stdout.starts_with("eq_1band EQ 1-Band Eq\n"), "{}", got.stdout); +} + +#[test] +fn app_reports_a_refused_connection_and_checks_arguments_first() { + for args in [ + vec!["app", "--port", "1", "session.inspect"], + vec!["app", "--port=1", "session.inspect"], + vec!["app", "session.inspect", "--port", "1"], + vec!["app", "--port", "1", "session.inspect", "{}"], + ] { + assert_refused(&invoke(&args), 1); + } + assert_usage_error( + &invoke(&["app", "--port", "1", "session.inspect", "{bad"]), + "invalid value '{bad' for '[JSON]': bad JSON: key must be a string at line 1 column 2", + ); + for bad in ["abc", "", "70000", ":80", "host:", "host:abc"] { + assert_usage_error( + &invoke(&["app", "--port", bad, "session.inspect"]), + &format!("invalid value '{bad}' for '--port

': expected a port, 0-65535, or host:port"), + ); + assert_usage_error(&invoke(&["mcp", "--connect", bad]), &format!("invalid value '{bad}' for '--connect ': expected a port")); + } +} + +#[test] +fn mcp_argument_errors_fail_before_reading_stdin() { + assert_usage_error(&invoke(&["mcp", "--connect"]), "a value is required for '--connect ' but none was supplied"); + assert_usage_error(&invoke(&["mcp", "--connect", "--demo"]), "a value is required for '--connect ' but none was supplied"); + for flag in ["--automation-read-root", "--automation-write-root"] { + let message = format!("a value is required for '{flag}

' but none was supplied"); + assert_usage_error(&invoke(&["mcp", flag]), &message); + assert_usage_error(&invoke(&["mcp", flag, "--demo"]), &message); + assert_usage_error(&invoke(&["mcp", &format!("{flag}=")]), &message); + } + assert_fails( + &invoke(&["mcp", "--automation-read-root", "missing-dir"]), + &format!("cannot open the automation read root `missing-dir`: {}", not_found()), + ); + assert_usage_error( + &invoke(&["mcp", "--automation-read-root", ".", "--connect", "1"]), + "the argument '--automation-read-root ' cannot be used with '--connect '", + ); +} diff --git a/apps/soundcraft-cli/tests/options.rs b/apps/soundcraft-cli/tests/options.rs index 5ca67031..11ed9070 100644 --- a/apps/soundcraft-cli/tests/options.rs +++ b/apps/soundcraft-cli/tests/options.rs @@ -58,7 +58,11 @@ fn convert_rejects_an_unknown_option_instead_of_writing_the_file() { for bad in ["--bitdepth", "--normalise", "--bogus"] { let o = run(&["convert", input.to_str().unwrap(), out.to_str().unwrap(), bad]); assert!(!o.status.success(), "{bad} should fail"); - assert!(String::from_utf8_lossy(&o.stderr).contains(&format!("unknown option {bad}")), "{bad}: {}", String::from_utf8_lossy(&o.stderr)); + assert!( + String::from_utf8_lossy(&o.stderr).contains(&format!("unexpected argument '{bad}' found")), + "{bad}: {}", + String::from_utf8_lossy(&o.stderr) + ); assert!(!out.exists(), "{bad} should not write the output"); } } @@ -71,7 +75,11 @@ fn convert_rejects_an_option_whose_value_is_missing() { for bad in ["--rate", "--bit-depth"] { let o = run(&["convert", input.to_str().unwrap(), out.to_str().unwrap(), bad]); assert!(!o.status.success(), "{bad} should fail"); - assert!(String::from_utf8_lossy(&o.stderr).contains(&format!("{bad} needs a value")), "{bad}: {}", String::from_utf8_lossy(&o.stderr)); + assert!( + String::from_utf8_lossy(&o.stderr).contains(&format!("a value is required for '{bad} ")), + "{bad}: {}", + String::from_utf8_lossy(&o.stderr) + ); assert!(!out.exists(), "{bad} should not write the output"); } } @@ -104,11 +112,11 @@ fn run_rejects_a_misspelled_or_value_less_bounce() { let out = dir.join("x.wav"); let o = run(&["run", "--cmd", "session.new", "--bounce"]); assert!(!o.status.success()); - assert!(String::from_utf8_lossy(&o.stderr).contains("--bounce needs a value"), "{}", String::from_utf8_lossy(&o.stderr)); + assert!(String::from_utf8_lossy(&o.stderr).contains("a value is required for '--bounce '"), "{}", String::from_utf8_lossy(&o.stderr)); let o = run(&["run", "--cmd", "session.new", "--bounces", out.to_str().unwrap()]); assert!(!o.status.success()); - assert!(String::from_utf8_lossy(&o.stderr).contains("unknown option --bounces"), "{}", String::from_utf8_lossy(&o.stderr)); + assert!(String::from_utf8_lossy(&o.stderr).contains("unexpected argument '--bounces' found"), "{}", String::from_utf8_lossy(&o.stderr)); assert!(!out.exists(), "nothing should be bounced"); } diff --git a/apps/soundcraft-web/Cargo.toml b/apps/soundcraft-web/Cargo.toml index 501c026a..d161499b 100644 --- a/apps/soundcraft-web/Cargo.toml +++ b/apps/soundcraft-web/Cargo.toml @@ -15,6 +15,7 @@ path = "src/main.rs" [target.'cfg(target_arch = "wasm32")'.dependencies] soundcraft-engine = { workspace = true } +soundcraft-audio-io.workspace = true soundcraft-ui-egui.workspace = true soundcraft-playback = { workspace = true } soundcraft-model = { workspace = true } diff --git a/apps/soundcraft-web/src/web.rs b/apps/soundcraft-web/src/web.rs index 0f3bf2e0..9cc84010 100644 --- a/apps/soundcraft-web/src/web.rs +++ b/apps/soundcraft-web/src/web.rs @@ -1,5 +1,6 @@ //! The browser shell: the eframe web runner around [`SoundApp`]. +use soundcraft_audio_io::FileFormat; use soundcraft_engine::{Engine, io}; use soundcraft_ui_egui::dialogs::Dialog; use soundcraft_ui_egui::sampler_window::SampleRead; @@ -67,8 +68,6 @@ enum Purpose { } const SESSION_EXTS: &[&str] = &[io::BUNDLE_EXTENSION, soundcraft_model::SESSION_EXTENSION]; -const AUDIO_EXTS: &[&str] = - &["wav", "wave", "bwf", "aif", "aiff", "aifc", "flac", "mp3", "ogg", "oga", "m4a", "m4b", "aac", "mp4", "caf", "mkv", "webm"]; const MIDI_EXTS: &[&str] = &["mid", "midi", "smf"]; impl Purpose { @@ -94,11 +93,11 @@ impl Purpose { } } - fn filter(self) -> (&'static str, &'static [&'static str]) { + fn filter(self) -> (&'static str, Vec<&'static str>) { match self { - Self::Session => ("SoundCraft Session", SESSION_EXTS), - Self::Audio => ("Audio", AUDIO_EXTS), - Self::Midi => ("MIDI", MIDI_EXTS), + Self::Session => ("SoundCraft Session", SESSION_EXTS.to_vec()), + Self::Audio => ("Audio", FileFormat::readable_extensions()), + Self::Midi => ("MIDI", MIDI_EXTS.to_vec()), } } } @@ -130,7 +129,7 @@ impl WebShell { let (inbox, ctx) = (self.inbox.clone(), ctx.clone()); wasm_bindgen_futures::spawn_local(async move { let (label, exts) = purpose.filter(); - let Some(file) = rfd::AsyncFileDialog::new().add_filter(label, exts).pick_file().await else { return }; + let Some(file) = rfd::AsyncFileDialog::new().add_filter(label, &exts).pick_file().await else { return }; let bytes = file.read().await; inbox.borrow_mut().push((purpose, file.file_name(), bytes)); ctx.request_repaint(); @@ -228,17 +227,15 @@ fn download(name: &str, bytes: &[u8]) -> Result<(), String> { let document = window.document().ok_or("no document")?; let parts = js_sys::Array::of1(&js_sys::Uint8Array::from(bytes)); let opts = web_sys::BlobPropertyBag::new(); - opts.set_type(match name.rsplit_once('.').map(|(_, x)| x.to_ascii_lowercase()).as_deref() { + let ext = name.rsplit_once('.').map(|(_, x)| x.to_ascii_lowercase()); + opts.set_type(match ext.as_deref() { Some("scraftz") => "application/zip", - Some("wav") => "audio/wav", - Some("aif" | "aiff") => "audio/aiff", - Some("flac") => "audio/flac", Some("mid" | "midi") => "audio/midi", Some("svg") => "image/svg+xml", Some("txt") => "text/plain", Some("json" | "scraft") => "application/json", Some("musicxml") => "application/vnd.recordare.musicxml+xml", - _ => "application/octet-stream", + ext => ext.and_then(FileFormat::from_extension).map_or("application/octet-stream", FileFormat::mime_type), }); let blob = web_sys::Blob::new_with_u8_array_sequence_and_options(&parts, &opts).map_err(js)?; let url = web_sys::Url::create_object_url_with_blob(&blob).map_err(js)?; diff --git a/apps/soundcraft/Cargo.toml b/apps/soundcraft/Cargo.toml index 113b37c9..bd6acc4c 100644 --- a/apps/soundcraft/Cargo.toml +++ b/apps/soundcraft/Cargo.toml @@ -16,8 +16,11 @@ asio = ["soundcraft-playback/asio"] [dependencies] soundcraft-engine.workspace = true +soundcraft-audio-io.workspace = true soundcraft-ui-egui.workspace = true soundcraft-playback.workspace = true +soundcraft-automation.workspace = true +clap.workspace = true eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "wayland", "x11", "links", "clipboard"] } egui = { workspace = true, features = ["default_fonts", "monochrome_emoji_fonts"] } serde_json.workspace = true diff --git a/apps/soundcraft/src/config.rs b/apps/soundcraft/src/config.rs index c576c240..b1a1c4fb 100644 --- a/apps/soundcraft/src/config.rs +++ b/apps/soundcraft/src/config.rs @@ -17,21 +17,13 @@ enum Platform { Other, } -/// SoundCraft's settings folder. A nonempty `$SOUNDCRAFT_CONFIG_DIR` overrides the platform default. -/// The override is the complete directory; no application-name suffix is appended. An explicitly -/// empty value disables the settings directory instead of falling back to a personal profile. -/// Otherwise `portable.txt` beside the executable selects `/SoundCraftData`, when that -/// folder can be written (a read-only medium falls back to the platform default). -pub fn config_dir() -> Option { - static DIR: std::sync::OnceLock> = std::sync::OnceLock::new(); - DIR.get_or_init(|| { - let exe = std::env::current_exe().ok(); - resolve(exe.as_deref().and_then(Path::parent)) - }) - .clone() -} - -fn resolve(exe_dir: Option<&Path>) -> Option { +/// SoundCraft's settings folder, resolved from the variable lookup `get` (see `Env::from_lookup`). +/// A nonempty `$SOUNDCRAFT_CONFIG_DIR` overrides the platform default and is the complete +/// directory; no application-name suffix is appended. An explicitly empty value means no settings +/// folder instead of a fallback to a personal profile (the documented exception to RFC-0012 rule +/// 1.7). Otherwise `portable.txt` beside the executable selects `/SoundCraftData`, when +/// that folder can be written (a read-only medium falls back to the platform default). +pub fn resolve_config_dir(get: impl Fn(&str) -> Option) -> Option { let platform = if cfg!(target_os = "macos") { Platform::Mac } else if cfg!(windows) { @@ -39,11 +31,12 @@ fn resolve(exe_dir: Option<&Path>) -> Option { } else { Platform::Other }; - config_dir_with(platform, |name| std::env::var_os(name), exe_dir) + let exe = std::env::current_exe().ok(); + config_dir_with(platform, get, exe.as_deref().and_then(Path::parent)) } fn config_dir_with(platform: Platform, env: impl Fn(&str) -> Option, exe_dir: Option<&Path>) -> Option { - if let Some(dir) = env("SOUNDCRAFT_CONFIG_DIR") { + if let Some(dir) = env(crate::env::CONFIG_DIR) { return (!dir.is_empty()).then(|| PathBuf::from(dir)); } if let Some(exe_dir) = exe_dir.filter(|d| d.join(PORTABLE_MARKER).is_file()) { diff --git a/apps/soundcraft/src/env.rs b/apps/soundcraft/src/env.rs new file mode 100644 index 00000000..bee06b30 --- /dev/null +++ b/apps/soundcraft/src/env.rs @@ -0,0 +1,154 @@ +//! The environment variables the app reads, read once at startup into [`Env`]. Library crates +//! never read these; `main` passes the values down. Platform variables (`HOME`, `DISPLAY`, ...) +//! and test-only ones stay with the code that needs them. +//! +//! `RUST_LOG` is not a field: the logger reads it ([`rust_log`]) before anything else, so that +//! the warnings [`Switch`] logs for a malformed value reach the log file. + +use std::ffi::OsString; +use std::path::PathBuf; + +use crate::switch::Switch; + +pub(crate) const CONFIG_DIR: &str = "SOUNDCRAFT_CONFIG_DIR"; +pub(crate) const NO_PREFS: &str = "SOUNDCRAFT_NO_PREFS"; +pub(crate) const NO_SYSTEM_FONTS: &str = "SOUNDCRAFT_NO_SYSTEM_FONTS"; +pub(crate) const WAYLAND: &str = "SOUNDCRAFT_WAYLAND"; +pub(crate) const RUST_LOG: &str = "RUST_LOG"; + +/// The variables and their `--help` lines, in the order `--help` lists them. +pub(crate) const VARS: &[(&str, &str)] = &[ + // Read by clap (`env =` on `--control`), not by `Env`. + ("SOUNDCRAFT_CONTROL_PORT", "The --control fallback: a port or host:port, ignored for another host"), + (CONFIG_DIR, "The settings folder (ui.json, Autosave, Presets, logs); empty: none (unlike the others, where empty counts as unset)"), + (NO_PREFS, "1 doesn't read or write preferences and logs to stderr only; 0 or unset is off"), + (NO_SYSTEM_FONTS, "1 uses the built-in fonts only; 0 or unset is off"), + (WAYLAND, "Linux: 1 keeps the native Wayland backend instead of XWayland; 0 or unset is off"), + (RUST_LOG, "Log filter, env_logger style: debug, warn,soundcraft_mix=trace"), +]; + +/// The `--help` Environment section, generated from [`VARS`]. +pub(crate) fn help() -> String { + let width = VARS.iter().map(|(name, _)| name.len()).max().unwrap_or(0); + let mut out = String::from("Environment:\n"); + for (name, line) in VARS { + out.push_str(&format!(" {name:, + pub(crate) no_prefs: Switch, + pub(crate) no_system_fonts: Switch, + #[cfg(any(test, all(unix, not(target_os = "macos"))))] + pub(crate) wayland: Switch, +} + +impl Env { + /// Reads the process environment. Call after the logger is installed, so malformed switches are logged. + pub(crate) fn read() -> Env { + Env::from_lookup(|name| std::env::var_os(name)) + } + + pub(crate) fn from_lookup(get: impl Fn(&str) -> Option) -> Env { + let switch = |name: &str| match get(name) { + None => Switch::Default, + Some(v) => Switch::parse(name, Some(&v.to_string_lossy())), + }; + Env { + config_dir: crate::config::resolve_config_dir(&get), + no_prefs: switch(NO_PREFS), + no_system_fonts: switch(NO_SYSTEM_FONTS), + #[cfg(any(test, all(unix, not(target_os = "macos"))))] + wayland: switch(WAYLAND), + } + } + + /// Runs without preferences (`SOUNDCRAFT_NO_PREFS`, agents' test runs) neither read nor write them. + pub(crate) fn prefs_enabled(&self) -> bool { + self.no_prefs != Switch::On + } + + pub(crate) fn system_fonts(&self) -> bool { + self.no_system_fonts != Switch::On + } + + /// Whether to run through XWayland: winit has no file drag-and-drop on Wayland, so dropping + /// files from the file manager showed a "no" cursor. Needs a nonempty `DISPLAY` + /// (`display`); `SOUNDCRAFT_WAYLAND=1` keeps the native backend. + #[cfg(any(test, all(unix, not(target_os = "macos"))))] + pub(crate) fn use_xwayland(&self, display: Option<&OsString>) -> bool { + display.is_some_and(|d| !d.is_empty()) && self.wayland != Switch::On + } +} + +/// The `RUST_LOG` filter, `None` when unset or blank. +pub(crate) fn rust_log() -> Option { + std::env::var(RUST_LOG).ok().filter(|s| !s.trim().is_empty()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn lookup(vars: &'static [(&'static str, &'static str)]) -> Env { + Env::from_lookup(|name| vars.iter().find(|(n, _)| *n == name).map(|(_, v)| OsString::from(v))) + } + + #[test] + fn variables_are_read_into_typed_fields() { + let env = lookup(&[(CONFIG_DIR, "/c"), (NO_PREFS, "yes"), (NO_SYSTEM_FONTS, "0"), (WAYLAND, "bogus")]); + assert_eq!(env.config_dir, Some(PathBuf::from("/c"))); + assert_eq!((env.no_prefs, env.no_system_fonts, env.wayland), (Switch::On, Switch::Off, Switch::Default)); + } + + #[test] + fn settings_folder_is_resolved_into_env() { + // Empty means no folder, even with a platform folder available. + assert_eq!(lookup(&[(CONFIG_DIR, ""), ("HOME", "/h"), ("XDG_CONFIG_HOME", "/x")]).config_dir, None); + assert_eq!(lookup(&[(CONFIG_DIR, "/c"), ("HOME", "/h")]).config_dir, Some(PathBuf::from("/c"))); + let unset = lookup(&[("HOME", "/h"), ("APPDATA", "/h"), ("XDG_CONFIG_HOME", "/x")]).config_dir; + assert!(unset.is_some_and(|d| d.ends_with("SoundCraft") || d.ends_with("soundcraft") || d.ends_with("SoundCraftData"))); + // Disabling preferences doesn't depend on the folder. + let env = lookup(&[(CONFIG_DIR, "/c"), (NO_PREFS, "1")]); + assert!(env.config_dir.is_some() && !env.prefs_enabled()); + } + + #[test] + fn no_prefs_and_no_system_fonts_turn_features_off() { + assert!(Env::default().prefs_enabled() && Env::default().system_fonts()); + let env = Env { no_prefs: Switch::On, no_system_fonts: Switch::On, ..Env::default() }; + assert!(!env.prefs_enabled() && !env.system_fonts()); + assert!(Env { no_prefs: Switch::Off, ..Env::default() }.prefs_enabled()); + } + + #[test] + fn xwayland_needs_a_display_and_wayland_not_forced() { + let display = OsString::from(":0"); + assert!(Env::default().use_xwayland(Some(&display))); + assert!(Env { wayland: Switch::Off, ..Env::default() }.use_xwayland(Some(&display))); + assert!(!Env { wayland: Switch::On, ..Env::default() }.use_xwayland(Some(&display))); + assert!(!Env::default().use_xwayland(None)); + assert!(!Env::default().use_xwayland(Some(&OsString::new()))); + } + + #[test] + fn help_lists_every_variable() { + let help = help(); + for (name, line) in VARS { + assert!(help.contains(name) && help.contains(line), "{name} missing from\n{help}"); + } + assert_eq!(help.matches("0/off/false/no").count(), 1, "{help}"); + } +} diff --git a/apps/soundcraft/src/launch_args.rs b/apps/soundcraft/src/launch_args.rs new file mode 100644 index 00000000..65c4af1c --- /dev/null +++ b/apps/soundcraft/src/launch_args.rs @@ -0,0 +1,223 @@ +//! The desktop app's command line, as clap types: `soundcraft --help` comes from these doc +//! comments, so the help and the parser can't disagree. Anything they don't declare is a usage +//! error naming it (exit status 2), as is a bad `--control` or `SOUNDCRAFT_CONTROL_PORT`. A file +//! whose name starts with `-` goes after `--`. Another host's `host:port` is a usage error from +//! `--control` and ignored from the variable ([`control_port`]). +//! +//! The control flag is `soundcraft-automation`'s, shared with `soundcraft-cli`. + +use std::ffi::OsString; +use std::path::PathBuf; + +use clap::builder::StyledStr; +use clap::error::{ContextKind, ContextValue, ErrorKind}; +use clap::parser::ValueSource; +use clap::{Command, CommandFactory, FromArgMatches, Parser}; +use soundcraft_automation::ControlTarget; +use soundcraft_automation::args::{ControlServerArgs, blank_env_is_unset, name_env_vars}; + +/// Opens SoundCraft with the given files +#[derive(Parser)] +#[command(name = "soundcraft", version = soundcraft_automation::args::long_version(), after_help = crate::env::help())] +pub(crate) struct LaunchArgs { + /// Sessions, audio and MIDI files to open. Put `--` before a name that starts with `-` + pub(crate) files: Vec, + /// Start with the demo session + #[arg(long, visible_alias = "sample")] + pub(crate) demo: bool, + /// Don't open an audio device + #[arg(long)] + pub(crate) no_audio: bool, + #[command(flatten)] + control: ControlServerArgs, + /// The port the control server starts on, from `control` ([`control_port`]). + #[arg(skip)] + pub(crate) control_port: Option, +} + +/// The command line, where an empty or blank variable counts as unset. +pub(crate) fn command() -> Command { + LaunchArgs::command().mut_args(blank_env_is_unset) +} + +/// Parses the launch arguments (without the program name) with `cmd`, from [`command`]. macOS +/// passes a process serial number (`-psn_…`) when an app is started from Finder on old versions: +/// it is dropped first, unless it comes after `--`. A bad value from a variable is a usage error +/// naming the variable. +pub(crate) fn parse(mut cmd: Command, args: impl IntoIterator) -> Result { + let mut options = true; + let args: Vec = args + .into_iter() + .filter(|a| { + options &= a != "--"; + !(options && a.to_str().is_some_and(|s| s.starts_with("-psn_"))) + }) + .collect(); + let argv = std::iter::once(OsString::from("soundcraft")).chain(args.iter().cloned()); + let matches = cmd.try_get_matches_from_mut(argv).map_err(|e| name_env_vars(e, &cmd, &args))?; + let mut launch = LaunchArgs::from_arg_matches(&matches)?; + launch.control_port = control_port(launch.control.control.take(), matches.value_source("control")).map_err(|e| e.into_clap(&mut cmd))?; + Ok(launch) +} + +/// The port the control server starts on, from `--control` or `SOUNDCRAFT_CONTROL_PORT` +/// (`source`). The server listens on 127.0.0.1 only, so it can't serve another host's +/// `host:port`. From the flag that is a usage error. The variable is `soundcraft-cli app`'s too and +/// may point it at another machine, so from there it is ignored with a warning and the app opens +/// without a control server. +fn control_port(value: Option, source: Option) -> Result, RemoteHost> { + match value { + None => Ok(None), + Some(target) if target.is_loopback() => Ok(Some(target.port)), + Some(target) if source == Some(ValueSource::EnvVariable) => { + log::warn!("SOUNDCRAFT_CONTROL_PORT={target} names another host, so no control server starts: it listens on 127.0.0.1 only"); + Ok(None) + } + Some(target) => Err(RemoteHost(target)), + } +} + +/// A `--control` `host:port` naming another host. +#[derive(Debug, PartialEq)] +struct RemoteHost(ControlTarget); + +impl RemoteHost { + /// clap's invalid-value error for `--control`, with the reason as a tip. + fn into_clap(self, cmd: &mut Command) -> clap::Error { + let flag = cmd.get_arguments().find(|a| a.get_id() == "control").map_or_else(|| "--control".to_owned(), ToString::to_string); + let tip = + StyledStr::from("the control server listens on 127.0.0.1 only: give a port, or host:port with a loopback host (127.0.0.1, localhost)"); + let mut error = clap::Error::new(ErrorKind::ValueValidation).with_cmd(cmd); + error.insert(ContextKind::InvalidArg, ContextValue::String(flag)); + error.insert(ContextKind::InvalidValue, ContextValue::String(self.0.to_string())); + error.insert(ContextKind::Suggested, ContextValue::StyledStrs(vec![tip])); + error.insert(ContextKind::Usage, ContextValue::StyledStr(cmd.render_usage())); + error + } +} + +#[cfg(test)] +mod tests { + use super::*; + use clap::error::ErrorKind; + + /// Parse without the variable, so one exported where the tests run can't change them (the + /// process tests in `tests/launch_flags.rs` cover it). + fn parse_args(args: &[&str]) -> Result { + parse(command().mut_args(|a| a.env(None::<&str>)), args.iter().map(OsString::from)) + } + + fn usage_error(args: &[&str]) -> String { + let Err(e) = parse_args(args) else { panic!("{args:?} parsed") }; + assert_eq!(e.exit_code(), 2, "{args:?}: {e}"); + e.to_string() + } + + #[test] + fn command_is_consistent() { + command().debug_assert(); + } + + #[test] + fn flags_and_files_are_parsed() { + let a = parse_args(&["--demo", "--no-audio", "--control", "50494", "a.scraft", "b.wav"]).unwrap(); + assert!(a.demo && a.no_audio); + assert_eq!(a.control_port, Some(50494)); + assert_eq!(a.files, [PathBuf::from("a.scraft"), PathBuf::from("b.wav")]); + assert!(parse_args(&["--sample"]).unwrap().demo); + let none = parse_args(&[]).unwrap(); + assert!(!none.demo && !none.no_audio && none.control_port.is_none() && none.files.is_empty()); + } + + #[test] + fn version_and_help_stop_parsing() { + for flag in ["--version", "-V"] { + let e = parse_args(&[flag, "--control"]).err().unwrap(); + assert_eq!((e.kind(), e.exit_code()), (ErrorKind::DisplayVersion, 0)); + assert_eq!(e.to_string(), format!("soundcraft {}\n", soundcraft_automation::args::long_version())); + } + let e = parse_args(&["-h", "--bogus"]).err().unwrap(); + assert_eq!((e.kind(), e.exit_code()), (ErrorKind::DisplayHelp, 0)); + let help = e.to_string(); + for text in ["--demo", "--sample", "--no-audio", "--control "] { + assert!(help.contains(text), "{text}: {help}"); + } + let help = command().render_help().to_string(); + assert!(help.contains("[env: SOUNDCRAFT_CONTROL_PORT"), "{help}"); + for var in ["SOUNDCRAFT_CONFIG_DIR", "SOUNDCRAFT_NO_PREFS", "SOUNDCRAFT_NO_SYSTEM_FONTS", "SOUNDCRAFT_WAYLAND", "RUST_LOG"] { + assert!(help.contains(&format!("\n {var} ")), "{var}: {help}"); + } + } + + #[test] + fn a_bad_control_port_is_a_usage_error_naming_it() { + assert!(usage_error(&["--control", "78787"]).contains("invalid value '78787' for '--control '")); + assert!(usage_error(&["--control", "nope"]).contains("invalid value 'nope'")); + assert!(usage_error(&["--control"]).contains("a value is required for '--control '")); + assert!(usage_error(&["--control", "--demo"]).contains("a value is required for '--control '")); + assert!(usage_error(&["--control", "-1"]).contains("invalid value '-1' for '--control '")); + let remote = usage_error(&["--control", "10.0.0.5:7979"]); + assert!( + remote.starts_with("error: invalid value '10.0.0.5:7979' for '--control '\n\n tip: the control server listens on 127.0.0.1 only"), + "{remote}" + ); + assert!(remote.contains("\nUsage: soundcraft "), "{remote}"); + assert!(usage_error(&["--control", "::1:7979"]).contains("invalid value '::1:7979' for '--control '")); + } + + /// The variable may point `soundcraft-cli app` at another machine: the app then opens without + /// a control server. + #[test] + fn another_host_is_ignored_only_from_the_variable() { + let remote = || ControlTarget { host: Some("10.0.0.5".to_owned()), port: 7979 }; + assert_eq!(control_port(Some(remote()), Some(ValueSource::EnvVariable)), Ok(None)); + assert_eq!(control_port(Some(remote()), Some(ValueSource::CommandLine)), Err(RemoteHost(remote()))); + let local = ControlTarget { host: None, port: 7979 }; + assert_eq!(control_port(Some(local), Some(ValueSource::EnvVariable)), Ok(Some(7979))); + assert_eq!(control_port(None, None), Ok(None)); + } + + /// The variable `soundcraft-cli app` reads may hold `host:port`: a loopback one gives its port. + #[test] + fn a_loopback_host_and_port_gives_its_port() { + for value in ["127.0.0.1:50494", "localhost:50494"] { + assert_eq!(parse_args(&["--control", value]).unwrap().control_port, Some(50494), "{value}"); + } + } + + /// The control server binds 127.0.0.1, so `[::1]` is another host. + #[test] + fn an_ipv6_loopback_host_is_not_loopback() { + assert!(usage_error(&["--control", "[::1]:50494"]).contains("invalid value '[::1]:50494' for '--control '")); + } + + #[test] + fn unknown_flags_and_repeats_are_usage_errors() { + assert!(usage_error(&["--bogus"]).contains("unexpected argument '--bogus'")); + assert!(usage_error(&["-x"]).contains("unexpected argument '-x'")); + assert!(usage_error(&["--demo", "--demo"]).contains("cannot be used multiple times")); + } + + #[test] + fn double_dash_ends_options() { + let a = parse_args(&["a.wav", "--", "--demo", "-x"]).unwrap(); + assert!(!a.demo); + assert_eq!(a.files, [PathBuf::from("a.wav"), PathBuf::from("--demo"), PathBuf::from("-x")]); + } + + #[test] + fn a_finder_process_serial_number_is_dropped() { + assert_eq!(parse_args(&["-psn_0_12345", "a.wav"]).unwrap().files, [PathBuf::from("a.wav")]); + assert_eq!(parse_args(&["--", "-psn_x.wav"]).unwrap().files, [PathBuf::from("-psn_x.wav")]); + } + + /// A file name that isn't valid Unicode reaches `files` as it is. + #[cfg(unix)] + #[test] + fn non_unicode_file_names_are_kept() { + use std::os::unix::ffi::OsStringExt as _; + let latin1 = OsString::from_vec(b"caf\xe9.wav".to_vec()); + let a = parse(command().mut_args(|a| a.env(None::<&str>)), [latin1.clone()]).unwrap(); + assert_eq!(a.files, [PathBuf::from(latin1)]); + } +} diff --git a/apps/soundcraft/src/logging.rs b/apps/soundcraft/src/logging.rs index c82c58ac..a040dd08 100644 --- a/apps/soundcraft/src/logging.rs +++ b/apps/soundcraft/src/logging.rs @@ -413,13 +413,12 @@ impl log::Log for AppLogger { fn flush(&self) {} } -/// Install the logger (filter from `RUST_LOG`, else [`DEFAULT_FILTER`]); `None` when another +/// Install the logger (filter from `rust_log`, the `RUST_LOG` value, else [`DEFAULT_FILTER`]); `None` when another /// logger was installed first. Call [`AppLogger::attach_dir`] or [`AppLogger::no_file`] once the /// settings directory is known. -pub fn install() -> Option<&'static AppLogger> { +pub fn install(rust_log: Option<&str>) -> Option<&'static AppLogger> { static LOGGER: OnceLock = OnceLock::new(); - let spec = std::env::var("RUST_LOG").ok().filter(|s| !s.trim().is_empty()); - let filter = Filter::parse(spec.as_deref().unwrap_or(DEFAULT_FILTER)); + let filter = Filter::parse(rust_log.unwrap_or(DEFAULT_FILTER)); let max = filter.max(); let logger = LOGGER.get_or_init(|| AppLogger::new(filter, true)); log::set_logger(logger).ok()?; diff --git a/apps/soundcraft/src/main.rs b/apps/soundcraft/src/main.rs index bffb9cb3..8e54a5bd 100644 --- a/apps/soundcraft/src/main.rs +++ b/apps/soundcraft/src/main.rs @@ -1,6 +1,6 @@ //! SoundCraft desktop app. //! -//! Usage: `soundcraft [--demo] [--control ] [--no-audio] [session.scraft | audio files…]` +//! Usage: `soundcraft [--demo] [--control ] [--no-audio] [session.scraft | audio files…]`; `soundcraft --help` lists the flags and `SOUNDCRAFT_CONTROL_PORT`. //! //! `--control ` (or `SOUNDCRAFT_CONTROL_PORT`) starts a localhost JSON-lines control server //! (`0` picks a free port and prints it): `{"id":1,"method":"ui.inspect","params":{}}` → @@ -13,17 +13,22 @@ mod config; mod control_server; +mod env; #[cfg(any(target_os = "windows", test))] mod graphics; +mod launch_args; mod logging; #[cfg(target_os = "macos")] mod native_menu; +mod switch; +use env::Env; +use soundcraft_audio_io::FileFormat; use soundcraft_engine::Engine; use soundcraft_ui_egui::{Services, SoundApp, UiState, dialogs}; use std::sync::Arc; -struct App(SoundApp, Option<&'static logging::AppLogger>, #[cfg(target_os = "macos")] Option); +struct App(SoundApp, Option<&'static logging::AppLogger>, Env, #[cfg(target_os = "macos")] Option); /// Sends the title only when it changes, remembered here as X11 and Wayland can't read it back. fn set_title(ctx: &egui::Context, title: String) { @@ -43,20 +48,20 @@ impl eframe::App for App { logger.report_audio_thread(); } #[cfg(target_os = "macos")] - if let Some(menu) = &mut self.2 { + if let Some(menu) = &mut self.3 { menu.process(&mut self.0); } self.0.guarded(|app| app.logic(ctx)); #[cfg(target_os = "macos")] - if let Some(menu) = &mut self.2 { + if let Some(menu) = &mut self.3 { menu.sync(&self.0); } // Files dropped on the window: audio on an instrument track or a Sampler window loads // the sampler; otherwise sessions open and audio/MIDI import. - let dropped: Vec = - self.0.take_dropped_files().iter().map(|f| f.path().to_string_lossy().into_owned()).filter(|s| !s.is_empty()).collect(); - for p in dropped { - open_path(&mut self.0, &p); + for f in self.0.take_dropped_files() { + if !f.path().as_os_str().is_empty() { + dialogs::open_path(&mut self.0, f.path()); + } } if self.0.quit_requested { ctx.send_viewport_cmd(egui::ViewportCommand::Close); @@ -72,7 +77,7 @@ impl eframe::App for App { self.0.ui.ui_scale = self.0.extra.ui_scale; // Saved now rather than at exit: the recent list matters most after a crash. if self.0.take_recent_changed() { - save_prefs(&self.0.ui); + save_prefs(&self.2, &self.0.ui); } // With the system menu bar there is no egui bar showing the window and dirty state, so the // title carries them; elsewhere the egui menu bar already does. @@ -98,7 +103,7 @@ impl eframe::App for App { } fn on_exit(&mut self) { self.0.close_autosave(); - save_prefs(&self.0.ui); + save_prefs(&self.2, &self.0.ui); if let Some(logger) = self.1 { logger.report_audio_thread(); } @@ -109,42 +114,16 @@ impl eframe::App for App { } } -fn open_path(app: &mut SoundApp, p: &str) { - let lower = p.to_ascii_lowercase(); - let r = if lower.ends_with(".scraft") { - dialogs::run_or_ask(app, "session.open", serde_json::json!({"path": p})) - } else if lower.ends_with(".mid") || lower.ends_with(".midi") || lower.ends_with(".smf") { - app.run("file.import_midi", serde_json::json!({"path": p})) - } else { - // Decoded in the background so a long file doesn't freeze the window (#597). - let at = app.engine.session().edit.selection.start; - soundcraft_ui_egui::import::start(app, serde_json::json!({"path": p, "at": at})); - Ok(serde_json::Value::Null) - }; - if let Err(e) = r { - log::warn!("{p}: {e}"); - } -} - -/// The per-user settings directory: `ui.json`, `Autosave/`, `Presets/` and `logs/` live here. -fn config_dir() -> Option { - config::config_dir() -} - -fn prefs_path() -> Option { - config_dir().map(|b| b.join("ui.json")) +fn prefs_path(env: &Env) -> Option { + env.config_dir.as_ref().map(|b| b.join("ui.json")) } /// Runs without preferences (`SOUNDCRAFT_NO_PREFS`, agents' test runs) neither read nor write them. -fn prefs_enabled() -> bool { - std::env::var_os("SOUNDCRAFT_NO_PREFS").is_none() -} - -fn load_prefs() -> Option { - if !prefs_enabled() { +fn load_prefs(env: &Env) -> Option { + if !env.prefs_enabled() { return None; } - let path = prefs_path()?; + let path = prefs_path(env)?; match config::load_ui(&path) { Ok(state) => state, Err(error) => { @@ -154,11 +133,11 @@ fn load_prefs() -> Option { } } -fn save_prefs(ui: &UiState) { - if !prefs_enabled() { +fn save_prefs(env: &Env, ui: &UiState) { + if !env.prefs_enabled() { return; } - if let Some(path) = prefs_path() + if let Some(path) = prefs_path(env) && let Err(error) = config::save_ui(&path, ui) { log::warn!("{error}"); @@ -172,17 +151,13 @@ fn services() -> Services { let d = match purpose { "session.open" => d.add_filter("SoundCraft Session", &["scraft"]), "file.import_midi" => d.add_filter("MIDI", &["mid", "midi", "smf"]), - _ => d.add_filter( - "Audio", - &["wav", "wave", "bwf", "aif", "aiff", "aifc", "flac", "mp3", "ogg", "oga", "m4a", "m4b", "aac", "mp4", "caf", "mkv", "webm"], - ), + _ => d.add_filter("Audio", &FileFormat::readable_extensions()), }; - d.pick_file().map(|p| p.to_string_lossy().into_owned()) + d.pick_file() })), - pick_save: Some(Box::new(|purpose: &str, default: &str| { - let path = std::path::Path::new(default); + pick_save: Some(Box::new(|purpose: &str, path: &std::path::Path| { let mut d = match purpose { - "file.bounce_mix" => rfd::FileDialog::new().add_filter("Audio", &["wav", "aif", "aiff", "flac"]), + "file.bounce_mix" => rfd::FileDialog::new().add_filter("Audio", &FileFormat::writable_extensions()), _ => rfd::FileDialog::new().add_filter("SoundCraft Session", &["scraft"]), }; if let Some(dir) = path.parent().filter(|dir| dir.is_dir()) { @@ -191,11 +166,9 @@ fn services() -> Services { if let Some(n) = path.file_name().and_then(std::ffi::OsStr::to_str) { d = d.set_file_name(n); } - d.save_file().map(|p| p.to_string_lossy().into_owned()) - })), - pick_folder: Some(Box::new(|start: &str| { - rfd::FileDialog::new().set_directory(start).pick_folder().map(|p| p.to_string_lossy().into_owned()) + d.save_file() })), + pick_folder: Some(Box::new(|start: &std::path::Path| rfd::FileDialog::new().set_directory(start).pick_folder())), } } @@ -217,34 +190,22 @@ fn main() -> eframe::Result { } // First, so every start-up record (and the engine's panic hook, installed with the first // Engine) is captured; see `logging`. - let logger = logging::install(); - let args: Vec = std::env::args().skip(1).collect(); - if args.iter().any(|a| a == "--version" || a == "-V") { - println!("SoundCraft {}", env!("CARGO_PKG_VERSION")); - return Ok(()); - } - let demo = args.iter().any(|a| a == "--demo" || a == "--sample"); - let no_audio = args.iter().any(|a| a == "--no-audio"); - let control_port: Option = args - .iter() - .position(|a| a == "--control") - .and_then(|i| args.get(i + 1)) - .and_then(|p| p.parse().ok()) - .or_else(|| std::env::var("SOUNDCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok())); - let files: Vec = args - .iter() - .enumerate() - .filter(|(i, a)| !(a.starts_with("--") || (i > &0 && args.get(i - 1).is_some_and(|p| p == "--control")))) - .map(|(_, a)| a.clone()) - .collect(); - log::info!("SoundCraft {} ({} {})", env!("CARGO_PKG_VERSION"), std::env::consts::OS, std::env::consts::ARCH); + let logger = logging::install(env::rust_log().as_deref()); + // Help, version and usage errors print and exit here, before a window or a log file exists. + let launch = launch_args::parse(launch_args::command(), std::env::args_os().skip(1)).unwrap_or_else(|e| e.exit()); + // After the arguments and the logger: reading resolves the settings folder, which creates it, + // and a malformed switch is logged. + let env = Env::read(); + let (demo, no_audio, control_port) = (launch.demo, launch.no_audio, launch.control_port); + let files = launch.files; + log::info!("SoundCraft {}", soundcraft_automation::args::long_version()); // The log file lives in the settings directory; opened after the arguments, so `--version` // leaves no file behind. Records logged until now are written to it first. Runs without // preferences (agents' test runs) log to standard error only, so they don't rotate away the // user's own logs. if let Some(logger) = logger { - match config_dir() { - _ if !prefs_enabled() => logger.no_file(), + match env.config_dir.as_deref() { + _ if !env.prefs_enabled() => logger.no_file(), Some(dir) => match logger.attach_dir(&dir.join(logging::LOG_DIR)) { Ok(path) => log::info!("log file {}", path.display()), Err(e) => log::warn!("no log file: {e}"), @@ -257,7 +218,7 @@ fn main() -> eframe::Result { } let engine = if demo { soundcraft_engine::demo::demo_engine() } else { Engine::default() }; - let prefs = load_prefs().unwrap_or_default(); + let prefs = load_prefs(&env).unwrap_or_default(); let mut options = eframe::NativeOptions { viewport: egui::ViewportBuilder::default() .with_title("SoundCraft") @@ -278,7 +239,7 @@ fn main() -> eframe::Result { // winit has no file drag-and-drop on Wayland (only on X11), so dropping files from the file // manager showed a "no" cursor. Run through XWayland when it's there; SOUNDCRAFT_WAYLAND=1 // keeps the native Wayland backend. - if std::env::var_os("DISPLAY").is_some_and(|d| !d.is_empty()) && std::env::var_os("SOUNDCRAFT_WAYLAND").is_none() { + if env.use_xwayland(std::env::var_os("DISPLAY").as_ref()) { options.event_loop_builder = Some(Box::new(|b| { use winit::platform::x11::EventLoopBuilderExtX11; b.with_x11(); @@ -292,9 +253,10 @@ fn main() -> eframe::Result { if no_audio { None } else { Some(soundcraft_playback::Player::with_config(Arc::new(engine.session().clone()), &prefs.audio)) }; let mut app = SoundApp::new(engine, player, services()); app.set_startup_update_check(true); - app.autosave_dir = prefs_path().and_then(|p| p.parent().map(|d| d.join("Autosave"))); + app.set_system_fonts(env.system_fonts()); + app.autosave_dir = env.config_dir.as_ref().map(|d| d.join("Autosave")); app.leftovers = app.autosave_dir.as_deref().map(soundcraft_engine::autosave::leftovers).unwrap_or_default(); - app.preset_dir = prefs_path().and_then(|p| p.parent().map(|d| d.join("Presets"))); + app.preset_dir = env.config_dir.as_ref().map(|d| d.join("Presets")); app.extra.ui_scale = prefs.ui_scale; app.ui = prefs; if let Some(port) = control_port { @@ -302,7 +264,7 @@ fn main() -> eframe::Result { app = app.with_control(rx); } for f in &files { - open_path(&mut app, f); + dialogs::open_path(&mut app, f); } // If the system menu bar cannot be built, keep the egui menu bar rather than fail to start. #[cfg(target_os = "macos")] @@ -319,6 +281,7 @@ fn main() -> eframe::Result { Ok(Box::new(App( app, logger, + env, #[cfg(target_os = "macos")] menu, ))) diff --git a/apps/soundcraft/src/switch.rs b/apps/soundcraft/src/switch.rs new file mode 100644 index 00000000..91cacb23 --- /dev/null +++ b/apps/soundcraft/src/switch.rs @@ -0,0 +1,46 @@ +//! Boolean environment switches (`SOUNDCRAFT_WAYLAND`, `SOUNDCRAFT_NO_PREFS`, ...): one parser, so every +//! variable accepts the same spellings. + +/// `1/on/true/yes` or `0/off/false/no`, case- and space-insensitive. Blank or unset is `Default`; +/// anything else is `Default` and logged. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) enum Switch { + On, + Off, + #[default] + Default, +} + +impl Switch { + /// Parse the value of the variable `name`; `name` only appears in the warning. + pub(crate) fn parse(name: &str, value: Option<&str>) -> Switch { + let Some(value) = value else { return Switch::Default }; + match value.trim().to_ascii_lowercase().as_str() { + "1" | "on" | "true" | "yes" => Switch::On, + "0" | "off" | "false" | "no" => Switch::Off, + "" => Switch::Default, + _ => { + log::warn!("{name}={value:?} is not 1/on/true/yes or 0/off/false/no; using the default"); + Switch::Default + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_every_spelling() { + for v in ["1", "on", "true", "yes", "ON", "True", " Yes ", "\t1\n"] { + assert_eq!(Switch::parse("X", Some(v)), Switch::On, "{v:?}"); + } + for v in ["0", "off", "false", "no", "OFF", "False", " No ", " 0 "] { + assert_eq!(Switch::parse("X", Some(v)), Switch::Off, "{v:?}"); + } + for v in [None, Some(""), Some(" "), Some("maybe"), Some("2"), Some("onn")] { + assert_eq!(Switch::parse("X", v), Switch::Default, "{v:?}"); + } + } +} diff --git a/apps/soundcraft/tests/launch_flags.rs b/apps/soundcraft/tests/launch_flags.rs new file mode 100644 index 00000000..f0400eb3 --- /dev/null +++ b/apps/soundcraft/tests/launch_flags.rs @@ -0,0 +1,55 @@ +//! The desktop binary's command line, run as a process: the variable fallback of `--control`. + +use std::process::{Command, Output}; + +fn soundcraft(args: &[&str], control_port: &str) -> Output { + Command::new(env!("CARGO_BIN_EXE_soundcraft")) + .args(args) + .env("SOUNDCRAFT_NO_PREFS", "1") + .env("SOUNDCRAFT_CONTROL_PORT", control_port) + .env_remove("RUST_LOG") + // Without a display the window cannot open, so a launch that passes parsing ends instead of + // staying up. + .env_remove("DISPLAY") + .env_remove("WAYLAND_DISPLAY") + .output() + .expect("run soundcraft") +} + +#[test] +fn a_bad_variable_is_a_usage_error_naming_it() { + let out = soundcraft(&["--no-audio"], "nope"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(2), "{stderr}"); + assert!(stderr.contains("invalid value 'nope' for 'SOUNDCRAFT_CONTROL_PORT'"), "{stderr}"); +} + +#[cfg(all(unix, not(target_os = "macos")))] +#[test] +fn a_blank_variable_is_unset() { + let out = soundcraft(&["--no-audio"], " "); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_ne!(out.status.code(), Some(2), "{stderr}"); + assert!(!stderr.contains("SOUNDCRAFT_CONTROL_PORT"), "{stderr}"); +} + +/// `soundcraft-cli app` takes `host:port` from the same variable. +#[cfg(all(unix, not(target_os = "macos")))] +#[test] +fn a_loopback_host_and_port_variable_is_accepted() { + let out = soundcraft(&["--no-audio"], "127.0.0.1:0"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_ne!(out.status.code(), Some(2), "{stderr}"); + assert!(!stderr.contains("SOUNDCRAFT_CONTROL_PORT"), "{stderr}"); +} + +/// The variable may point `soundcraft-cli app` at another machine: the app opens, with a warning +/// and no control server. +#[cfg(all(unix, not(target_os = "macos")))] +#[test] +fn another_host_in_the_variable_is_ignored() { + let out = soundcraft(&["--no-audio"], "10.0.0.5:7979"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_ne!(out.status.code(), Some(2), "{stderr}"); + assert!(stderr.contains("SOUNDCRAFT_CONTROL_PORT=10.0.0.5:7979 names another host, so no control server starts"), "{stderr}"); +} diff --git a/crates/audio-io/src/lib.rs b/crates/audio-io/src/lib.rs index f47f244a..bdb9d352 100644 --- a/crates/audio-io/src/lib.rs +++ b/crates/audio-io/src/lib.rs @@ -61,9 +61,110 @@ pub enum FileFormat { Aac, Alac, Caf, + /// Any other container `symphonia` reads: Matroska and WebM by name. Other, } +impl FileFormat { + /// The formats [`decode`] reads, found by extension. Contents decide when they disagree. + pub const READ: &'static [FileFormat] = + &[FileFormat::Wav, FileFormat::Aiff, FileFormat::Flac, FileFormat::Mp3, FileFormat::Ogg, FileFormat::Aac, FileFormat::Caf, FileFormat::Other]; + /// The formats [`encode`] writes. + pub const WRITE: &'static [FileFormat] = &[FileFormat::Wav, FileFormat::Aiff, FileFormat::Flac]; + + /// The extensions that name this format, lower case and without the dot; the first is the one + /// a new file gets. ALAC shares AAC's MP4 container, so a `.m4a` name reads as AAC until the + /// contents say otherwise. + pub fn extensions(self) -> &'static [&'static str] { + match self { + FileFormat::Wav => &["wav", "wave", "bwf", "rf64"], + FileFormat::Aiff => &["aif", "aiff", "aifc"], + FileFormat::Flac => &["flac"], + FileFormat::Mp3 => &["mp3", "mp2", "mp1"], + FileFormat::Ogg => &["ogg", "oga"], + FileFormat::Aac => &["m4a", "m4b", "aac", "mp4", "adts"], + FileFormat::Alac => &["m4a"], + FileFormat::Caf => &["caf"], + FileFormat::Other => &["mkv", "webm"], + } + } + + /// The name people know the format by, as menus show it. + pub fn name(self) -> &'static str { + match self { + FileFormat::Wav => "WAV", + FileFormat::Aiff => "AIFF", + FileFormat::Flac => "FLAC", + FileFormat::Mp3 => "MP3", + FileFormat::Ogg => "Ogg Vorbis", + FileFormat::Aac => "AAC", + FileFormat::Alac => "ALAC", + FileFormat::Caf => "CAF", + FileFormat::Other => "Other", + } + } + + /// The media type a file of this format is served or downloaded as. + pub fn mime_type(self) -> &'static str { + match self { + FileFormat::Wav => "audio/wav", + FileFormat::Aiff => "audio/aiff", + FileFormat::Flac => "audio/flac", + FileFormat::Mp3 => "audio/mpeg", + FileFormat::Ogg => "audio/ogg", + FileFormat::Aac | FileFormat::Alac => "audio/mp4", + FileFormat::Caf => "audio/x-caf", + FileFormat::Other => "application/octet-stream", + } + } + + /// The extension a new file of this format gets. + pub fn default_extension(self) -> &'static str { + self.extensions().first().copied().unwrap_or("bin") + } + + /// The readable format an extension or format name names (`wav`, `.AIFF`; ASCII + /// case-insensitive), or `None` when nothing reads it. + pub fn from_extension(ext: &str) -> Option { + let ext = ext.trim_start_matches('.'); + Self::READ.iter().copied().find(|f| f.extensions().iter().any(|x| x.eq_ignore_ascii_case(ext))) + } + + /// The readable format `path`'s extension names, or `None` without a known extension. + pub fn from_path(path: impl AsRef) -> Option { + path.as_ref().extension().and_then(std::ffi::OsStr::to_str).and_then(Self::from_extension) + } + + /// The extensions [`encode`] writes this format under: the names whose readers expect what it + /// writes. RF64 and AIFF-C names are left out because `encode` writes plain RIFF and AIFF + /// headers for them. + pub fn write_extensions(self) -> &'static [&'static str] { + match self { + FileFormat::Wav => &["wav", "wave", "bwf"], + FileFormat::Aiff => &["aif", "aiff"], + FileFormat::Flac => &["flac"], + _ => &[], + } + } + + /// The format [`encode`] writes for an extension or format name (ASCII case-insensitive), or + /// `None` when it writes none. + pub fn from_write_extension(ext: &str) -> Option { + let ext = ext.trim_start_matches('.'); + Self::WRITE.iter().copied().find(|f| f.write_extensions().iter().any(|x| x.eq_ignore_ascii_case(ext))) + } + + /// Every extension of [`FileFormat::READ`], in order: what file pickers and browsers filter by. + pub fn readable_extensions() -> Vec<&'static str> { + Self::READ.iter().flat_map(|f| f.extensions().iter().copied()).collect() + } + + /// Every extension of [`FileFormat::WRITE`], in order: what save dialogs, help and errors list. + pub fn writable_extensions() -> Vec<&'static str> { + Self::WRITE.iter().flat_map(|f| f.write_extensions().iter().copied()).collect() + } +} + /// Stored sample representation. #[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub enum SampleFormat { @@ -192,22 +293,9 @@ fn detect_by_magic(b: &[u8], depth: u32) -> Option { None } -fn detect_by_ext(ext: &str) -> FileFormat { - match ext.trim_start_matches('.').to_ascii_lowercase().as_str() { - "wav" | "wave" | "bwf" | "rf64" => FileFormat::Wav, - "aif" | "aiff" | "aifc" => FileFormat::Aiff, - "flac" => FileFormat::Flac, - "mp3" | "mp2" | "mp1" => FileFormat::Mp3, - "ogg" | "oga" => FileFormat::Ogg, - "aac" | "m4a" | "m4b" | "mp4" | "adts" => FileFormat::Aac, - "caf" => FileFormat::Caf, - _ => FileFormat::Other, - } -} - /// Identify a file by its magic bytes, falling back to the extension hint (`"wav"`, `".flac"`). pub fn detect_format(bytes: &[u8], ext_hint: Option<&str>) -> FileFormat { - detect_by_magic(bytes, 0).unwrap_or_else(|| ext_hint.map(detect_by_ext).unwrap_or(FileFormat::Other)) + detect_by_magic(bytes, 0).unwrap_or_else(|| ext_hint.and_then(FileFormat::from_extension).unwrap_or(FileFormat::Other)) } /// Read a file's format, sample rate, channel count, length and BWF metadata. @@ -270,19 +358,6 @@ pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { } } -/// Formats [`encode`] can write, as the extensions that name them. -pub const ENCODE_EXTENSIONS: &str = "wav, aif, aiff, flac"; - -/// The format [`encode`] writes for a file extension or format name (`wav`, `aiff`, `flac`, case-insensitive), or `None` when it cannot write it. -pub fn encode_format_for(name: &str) -> Option { - match name.trim_start_matches('.').to_ascii_lowercase().as_str() { - "wav" | "wave" | "bwf" => Some(FileFormat::Wav), - "aif" | "aiff" => Some(FileFormat::Aiff), - "flac" => Some(FileFormat::Flac), - _ => None, - } -} - /// Encode a WAV file in the RF64 layout regardless of size (normally used only above 4 GiB). pub fn encode_wav_rf64(buf: &AudioBuffer, opts: &EncodeOptions) -> Result> { wav::encode(buf, opts.bit_depth, opts.dither, opts.bwf.as_ref(), true, None) diff --git a/crates/audio-io/tests/fixtures/README.md b/crates/audio-io/tests/fixtures/README.md new file mode 100644 index 00000000..3cc0985f --- /dev/null +++ b/crates/audio-io/tests/fixtures/README.md @@ -0,0 +1,17 @@ +Tiny fixtures for `tests/it/decode_read_list.rs`: one per container or codec the READ list names +that the crate cannot write itself. Made with ffmpeg 8.0 from a 0.1 s mono 440 Hz sine +(`F="-y -f lavfi -i sine=frequency=440:duration=0.1 -ac 1"`): + + ffmpeg $F -ar 8000 -c:a libmp3lame -b:a 16k tone.mp3 + ffmpeg $F -ar 24000 -c:a mp2 -b:a 32k tone.mp2 + ffmpeg $F -ar 8000 -c:a libvorbis -q:a -1 tone.ogg + ffmpeg $F -ar 8000 -c:a aac -b:a 16k tone.m4a + ffmpeg $F -ar 8000 -c:a aac -b:a 16k -f adts tone.aac + ffmpeg $F -ar 8000 -c:a pcm_s16le tone.caf + ffmpeg $F -ar 8000 -c:a pcm_s16le tone.mkv + ffmpeg $F -ar 8000 -c:a libvorbis -q:a -1 tone.webm + ffmpeg $F -ar 8000 -c:a pcm_s16le -f aiff tone.aifc # AIFF-C "sowt" + +`silence.mp1` has no ffmpeg encoder. It is four hand-built MPEG-1 Layer I frames (mono, 32 kHz, +32 kbit/s, 48 bytes each: header `FF FF 18 C0`, all subband allocations zero), which decode to +silence. diff --git a/crates/audio-io/tests/fixtures/silence.mp1 b/crates/audio-io/tests/fixtures/silence.mp1 new file mode 100644 index 00000000..b2641229 Binary files /dev/null and b/crates/audio-io/tests/fixtures/silence.mp1 differ diff --git a/crates/audio-io/tests/fixtures/tone.aac b/crates/audio-io/tests/fixtures/tone.aac new file mode 100644 index 00000000..98cb40c0 Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.aac differ diff --git a/crates/audio-io/tests/fixtures/tone.aifc b/crates/audio-io/tests/fixtures/tone.aifc new file mode 100644 index 00000000..628dee16 Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.aifc differ diff --git a/crates/audio-io/tests/fixtures/tone.caf b/crates/audio-io/tests/fixtures/tone.caf new file mode 100644 index 00000000..2d3707ee Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.caf differ diff --git a/crates/audio-io/tests/fixtures/tone.m4a b/crates/audio-io/tests/fixtures/tone.m4a new file mode 100644 index 00000000..8966d86f Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.m4a differ diff --git a/crates/audio-io/tests/fixtures/tone.mkv b/crates/audio-io/tests/fixtures/tone.mkv new file mode 100644 index 00000000..eaeab78e Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.mkv differ diff --git a/crates/audio-io/tests/fixtures/tone.mp2 b/crates/audio-io/tests/fixtures/tone.mp2 new file mode 100644 index 00000000..a324d263 Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.mp2 differ diff --git a/crates/audio-io/tests/fixtures/tone.mp3 b/crates/audio-io/tests/fixtures/tone.mp3 new file mode 100644 index 00000000..c6429ac0 Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.mp3 differ diff --git a/crates/audio-io/tests/fixtures/tone.ogg b/crates/audio-io/tests/fixtures/tone.ogg new file mode 100644 index 00000000..5ebd4a7d Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.ogg differ diff --git a/crates/audio-io/tests/fixtures/tone.webm b/crates/audio-io/tests/fixtures/tone.webm new file mode 100644 index 00000000..5dbbcfee Binary files /dev/null and b/crates/audio-io/tests/fixtures/tone.webm differ diff --git a/crates/audio-io/tests/it/decode_read_list.rs b/crates/audio-io/tests/it/decode_read_list.rs new file mode 100644 index 00000000..ae477ad9 --- /dev/null +++ b/crates/audio-io/tests/it/decode_read_list.rs @@ -0,0 +1,51 @@ +//! Every extension the format table reads decodes with the compiled-in `symphonia` features. + +use soundcraft_audio_io::{AudioBuffer, BitDepth, EncodeOptions, FileFormat, decode, encode, probe}; + +fn fixture(name: &str) -> Vec { + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures").join(name); + std::fs::read(&path).unwrap_or_else(|e| panic!("{}: {e}", path.display())) +} + +fn generated(format: FileFormat) -> Vec { + let tone: Vec = (0..800).map(|i| (i as f32 * 0.1).sin() * 0.5).collect(); + let buf = AudioBuffer { sample_rate: 8000, channels: vec![tone] }; + encode(&buf, &EncodeOptions { format, bit_depth: BitDepth::Int16, dither: false, bwf: None }).unwrap() +} + +fn bytes_for(ext: &str) -> Vec { + match ext { + "wav" | "wave" | "bwf" => generated(FileFormat::Wav), + "rf64" => { + let tone = AudioBuffer { sample_rate: 8000, channels: vec![vec![0.25; 800]] }; + let opts = EncodeOptions { format: FileFormat::Wav, bit_depth: BitDepth::Int16, dither: false, bwf: None }; + soundcraft_audio_io::encode_wav_rf64(&tone, &opts).unwrap() + } + "aif" | "aiff" => generated(FileFormat::Aiff), + "aifc" => fixture("tone.aifc"), + "flac" => generated(FileFormat::Flac), + "mp3" => fixture("tone.mp3"), + "mp2" => fixture("tone.mp2"), + "mp1" => fixture("silence.mp1"), + "ogg" | "oga" => fixture("tone.ogg"), + "m4a" | "m4b" | "mp4" => fixture("tone.m4a"), + "aac" | "adts" => fixture("tone.aac"), + "caf" => fixture("tone.caf"), + "mkv" => fixture("tone.mkv"), + "webm" => fixture("tone.webm"), + other => panic!("no fixture for the READ extension {other:?}: add one"), + } +} + +#[test] +fn every_read_extension_probes_and_decodes() { + for &format in FileFormat::READ { + for ext in format.extensions() { + let bytes = bytes_for(ext); + let info = probe(&bytes, Some(ext)).unwrap_or_else(|e| panic!("probe .{ext}: {e}")); + let (_, buf) = decode(&bytes, Some(ext)).unwrap_or_else(|e| panic!("decode .{ext}: {e}")); + assert!(info.channels >= 1, ".{ext}"); + assert!(buf.frames() > 0, ".{ext}: no frames"); + } + } +} diff --git a/crates/audio-io/tests/it/encode_format.rs b/crates/audio-io/tests/it/encode_format.rs deleted file mode 100644 index 766d85b8..00000000 --- a/crates/audio-io/tests/it/encode_format.rs +++ /dev/null @@ -1,13 +0,0 @@ -use soundcraft_audio_io::{FileFormat, encode_format_for}; - -#[test] -fn only_formats_the_encoder_writes_are_named() { - assert_eq!(encode_format_for("wav"), Some(FileFormat::Wav)); - assert_eq!(encode_format_for("WAV"), Some(FileFormat::Wav)); - assert_eq!(encode_format_for(".aif"), Some(FileFormat::Aiff)); - assert_eq!(encode_format_for("aiff"), Some(FileFormat::Aiff)); - assert_eq!(encode_format_for("flac"), Some(FileFormat::Flac)); - for other in ["mp3", "ogg", "m4a", "xyz", ""] { - assert_eq!(encode_format_for(other), None, "{other}"); - } -} diff --git a/crates/audio-io/tests/it/file_format.rs b/crates/audio-io/tests/it/file_format.rs new file mode 100644 index 00000000..5e063a52 --- /dev/null +++ b/crates/audio-io/tests/it/file_format.rs @@ -0,0 +1,36 @@ +use soundcraft_audio_io::FileFormat; + +#[test] +fn every_read_and_write_format_round_trips_through_its_extensions() { + for &format in FileFormat::READ { + assert!(!format.extensions().is_empty(), "{format:?}"); + for ext in format.extensions() { + assert_eq!(FileFormat::from_extension(ext), Some(format), "{ext}"); + assert_eq!(FileFormat::from_extension(&format!(".{}", ext.to_ascii_uppercase())), Some(format), "{ext}"); + assert_eq!(FileFormat::from_path(format!("dir.x/Take 1.{ext}")), Some(format), "{ext}"); + } + } + for &format in FileFormat::WRITE { + assert!(!format.write_extensions().is_empty(), "{format:?}"); + for ext in format.write_extensions() { + assert_eq!(FileFormat::from_write_extension(ext), Some(format), "{ext}"); + assert_eq!(FileFormat::from_write_extension(&ext.to_ascii_uppercase()), Some(format), "{ext}"); + assert_eq!(FileFormat::from_extension(ext), Some(format), "{ext}: what is written reads back"); + } + assert!(format.write_extensions().contains(&format.default_extension()), "{format:?}"); + } +} + +#[test] +fn only_names_whose_readers_expect_the_encoder_output_are_writable() { + assert_eq!(FileFormat::writable_extensions(), ["wav", "wave", "bwf", "aif", "aiff", "flac"]); + for read_only in ["rf64", "aifc", "mp3", "ogg", "m4a", "mkv"] { + assert!(FileFormat::from_extension(read_only).is_some(), "{read_only}"); + assert_eq!(FileFormat::from_write_extension(read_only), None, "{read_only}"); + } + for unknown in ["xyz", "", "mid"] { + assert_eq!(FileFormat::from_extension(unknown), None, "{unknown}"); + } + assert_eq!(FileFormat::from_path("no-extension"), None); + assert_eq!(FileFormat::Aiff.default_extension(), "aif"); +} diff --git a/crates/audio-io/tests/it/main.rs b/crates/audio-io/tests/it/main.rs index 84b90aa1..d71617af 100644 --- a/crates/audio-io/tests/it/main.rs +++ b/crates/audio-io/tests/it/main.rs @@ -1,6 +1,7 @@ //! Integration tests, built as one test binary: each file is a module. -mod encode_format; +mod decode_read_list; +mod file_format; mod float_headroom; mod malformed; mod roundtrip; diff --git a/crates/automation/Cargo.toml b/crates/automation/Cargo.toml index 2e3f3f85..f83234a0 100644 --- a/crates/automation/Cargo.toml +++ b/crates/automation/Cargo.toml @@ -9,9 +9,15 @@ repository.workspace = true [dependencies] soundcraft-engine.workspace = true +soundcraft-audio-io.workspace = true soundcraft-time.workspace = true serde_json.workspace = true log.workspace = true +thiserror.workspace = true + +# The launch flags (`args`): the web build has none. +[target.'cfg(not(target_arch = "wasm32"))'.dependencies] +clap.workspace = true [lints] workspace = true diff --git a/crates/automation/src/args.rs b/crates/automation/src/args.rs new file mode 100644 index 00000000..3822bff5 --- /dev/null +++ b/crates/automation/src/args.rs @@ -0,0 +1,199 @@ +//! The control and automation launch options, as clap types, so each flag and its environment +//! variable are declared once for `soundcraft-cli` and the desktop app. A flag wins over its +//! variable. +//! +//! Parse with [`blank_env_is_unset`] applied to the arguments, so an empty variable counts as unset, +//! and pass a parse error through [`name_env_vars`], so a bad value names the variable it came from. +//! [`conflict`] reports two arguments that can't go together, each named as it was given, for a +//! conflict that depends on the mode, which clap's `conflicts_with` can't express. + +use std::ffi::OsString; +use std::path::PathBuf; +use std::sync::OnceLock; + +use clap::error::{ContextKind, ContextValue, ErrorKind}; +use clap::parser::ValueSource; +use clap::{Arg, ArgAction, ArgMatches, Args, Command}; + +use crate::ControlTarget; + +/// The running app a client calls (`soundcraft-cli app`). +#[derive(Args)] +pub struct ControlClientArgs { + /// The app's control port or host:port + #[arg(long, value_name = "P", env = "SOUNDCRAFT_CONTROL_PORT", default_value = "7979", value_parser = control_target)] + pub port: ControlTarget, +} + +/// The control server the desktop app starts. The same variable as [`ControlClientArgs`], so one +/// export connects the two. +#[derive(Args)] +pub struct ControlServerArgs { + /// Start the localhost JSON-lines control server on this port (0 picks a free port and prints + /// it) + // allow_negative_numbers makes `--control -1` a bad port rather than an unexpected argument. + #[arg(long, value_name = "PORT", env = "SOUNDCRAFT_CONTROL_PORT", value_parser = control_target, allow_negative_numbers = true)] + pub control: Option, +} + +/// The folders automation may touch. Once any root is given, files are confined to the roots, and +/// a kind left out grants none of its access. No variable stands in for them: one left exported +/// must not change what an agent may read or write. +#[derive(Args)] +pub struct AutomationArgs { + /// A folder the agent's commands may read from; repeat for several + #[arg(long, value_name = "DIR", action = ArgAction::Append)] + pub automation_read_root: Vec, + /// A folder the agent's commands may write to; repeat for several + #[arg(long, value_name = "DIR", action = ArgAction::Append)] + pub automation_write_root: Vec, +} + +/// A [`ControlTarget`], for clap's `value_parser`: `soundcraft-cli app --port`, `mcp --connect`, +/// `soundcraft --control`, and SOUNDCRAFT_CONTROL_PORT, which may carry spaces. +pub fn control_target(s: &str) -> Result { + let (host, port) = host_and_port(s.trim())?; + Ok(ControlTarget { host: host.map(str::to_owned), port }) +} + +/// `PORT` or `HOST:PORT`, split. An IPv6 host goes in brackets (`[::1]:7979`), as a socket address +/// is written: in `::1:7979` the host and port can't be told apart. +fn host_and_port(s: &str) -> Result<(Option<&str>, u16), NotATarget> { + let (host, port) = match s.rsplit_once(':') { + Some(("", _)) => return Err(NotATarget), + Some((host, _)) if host.contains(':') && !(host.starts_with('[') && host.ends_with(']')) => return Err(NotATarget), + Some((host, port)) => (Some(host), port), + None => (None, s), + }; + Ok((host, port.parse().map_err(|_| NotATarget)?)) +} + +/// A [`control_target`] value that is neither a port nor `host:port`. +#[derive(Debug, PartialEq, Eq, thiserror::Error)] +#[error("expected a port, 0-65535, or host:port")] +pub struct NotATarget; + +/// `0.1.0 (linux x86_64)`: the workspace version and the platform the binary was built for. Every +/// crate takes its version from the workspace, so the value is the same in both binaries. A +/// `const` can't build it: `concat!` accepts only literals, not `std::env::consts`. +pub fn long_version() -> &'static str { + static LONG: OnceLock = OnceLock::new(); + LONG.get_or_init(|| format!("{} ({} {})", env!("CARGO_PKG_VERSION"), std::env::consts::OS, std::env::consts::ARCH)) +} + +/// For `Command::mut_args`: an empty or blank variable counts as unset, so `SOUNDCRAFT_CONTROL_PORT=` +/// turns a fallback off without `unset`. clap would pass the empty value to the parser. +pub fn blank_env_is_unset(arg: Arg) -> Arg { + let blank = arg.get_env().and_then(std::env::var_os).is_some_and(|v| v.to_str().is_some_and(|v| v.trim().is_empty())); + if blank { arg.env(None::<&str>) } else { arg } +} + +/// clap names the flag when a value from its variable is bad or conflicts with another, as if the +/// flag had been given: name the variable instead when the flag isn't among `args` (without the +/// binary name; a first one naming a subcommand selects whose arguments to look in). +pub fn name_env_vars(mut error: clap::Error, cmd: &Command, args: &[OsString]) -> clap::Error { + // `--flag value` or `--flag=value`. + let given = + |long: &str| args.iter().filter_map(|a| a.to_str()?.strip_prefix("--")?.strip_prefix(long)).any(|r| r.is_empty() || r.starts_with('=')); + let sub = args.first().and_then(|name| cmd.find_subcommand(name)).unwrap_or(cmd); + for kind in [ContextKind::InvalidArg, ContextKind::PriorArg] { + let Some(ContextValue::String(shown)) = error.get(kind) else { continue }; + let Some(arg) = sub.get_arguments().find(|a| a.to_string() == *shown) else { continue }; + let (Some(long), Some(var)) = (arg.get_long(), arg.get_env()) else { continue }; + if !given(long) { + error.insert(kind, ContextValue::String(var.to_string_lossy().into_owned())); + } + } + error +} + +/// clap's own conflict error when `m`, parsed by `cmd`, has both `a` and `b`, each named as it was +/// given: the flag, or its variable. +pub fn conflict(cmd: &mut Command, m: &ArgMatches, a: &str, b: &str) -> Result<(), clap::Error> { + if m.value_source(a).is_none() || m.value_source(b).is_none() { + return Ok(()); + } + let shown = |id: &str| { + let arg = cmd.get_arguments().find(|arg| arg.get_id() == id); + match (m.value_source(id), arg.and_then(Arg::get_env)) { + (Some(ValueSource::EnvVariable), Some(var)) => var.to_string_lossy().into_owned(), + _ => arg.map_or_else(|| id.to_owned(), |arg| arg.to_string()), + } + }; + let (shown_a, shown_b) = (shown(a), shown(b)); + let mut error = clap::Error::new(ErrorKind::ArgumentConflict).with_cmd(cmd); + error.insert(ContextKind::InvalidArg, ContextValue::String(shown_a)); + error.insert(ContextKind::PriorArg, ContextValue::String(shown_b)); + error.insert(ContextKind::Usage, ContextValue::StyledStr(cmd.render_usage())); + Err(error) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn control_target_is_a_port_or_host_and_port() { + let target = |host: Option<&str>, port| Ok(ControlTarget { host: host.map(str::to_owned), port }); + assert_eq!(control_target("7979"), target(None, 7979)); + assert_eq!(control_target(" 0 "), target(None, 0)); + assert_eq!(control_target("localhost:7979 "), target(Some("localhost"), 7979)); + assert_eq!(control_target("10.0.0.5:1"), target(Some("10.0.0.5"), 1)); + assert_eq!(control_target("[::1]:7979"), target(Some("[::1]"), 7979)); + for bad in ["", "x", "78787", ":7979", "host:", "host:x", "-1", "::1:7979", "[::1:7979"] { + assert_eq!(control_target(bad), Err(NotATarget), "{bad:?}"); + } + } + + /// The control server listens on 127.0.0.1 only: `[::1]` is another address. + #[test] + fn a_loopback_target_is_a_port_127_0_0_1_or_localhost() { + for loopback in ["50494", "127.0.0.1:7979", "localhost:7979"] { + assert!(control_target(loopback).is_ok_and(|t| t.is_loopback()), "{loopback:?}"); + } + for remote in ["10.0.0.5:7979", "example.com:7979", "[::1]:7979"] { + assert!(control_target(remote).is_ok_and(|t| !t.is_loopback()), "{remote:?}"); + } + } + + #[test] + fn a_control_target_displays_as_a_socket_address() { + for (given, shown) in [("7979", "127.0.0.1:7979"), ("localhost:1", "localhost:1"), ("[::1]:7979", "[::1]:7979")] { + assert_eq!(control_target(given).map(|t| t.to_string()), Ok(shown.to_owned()), "{given:?}"); + } + } + + /// A command with one port flag backed by a variable no test sets. + fn port_command() -> Command { + Command::new("t").arg(Arg::new("port").long("port").env("SOUNDCRAFT_TEST_UNSET_PORT").value_parser(control_target)) + } + + #[test] + fn name_env_vars_names_the_variable_only_when_the_flag_was_not_given() { + let mut cmd = port_command(); + let e = cmd.try_get_matches_from_mut(["t", "--port", "x"]).unwrap_err(); + let flag = name_env_vars(e, &cmd, &["--port".into(), "x".into()]); + assert_eq!(flag.get(ContextKind::InvalidArg), Some(&ContextValue::String("--port ".into()))); + let e = cmd.try_get_matches_from_mut(["t", "--port=x"]).unwrap_err(); + // As the error from the variable would read: the flag is not among the arguments. + let var = name_env_vars(e, &cmd, &[]); + assert_eq!(var.get(ContextKind::InvalidArg), Some(&ContextValue::String("SOUNDCRAFT_TEST_UNSET_PORT".into()))); + } + + #[test] + fn blank_env_is_unset_keeps_an_unset_variable() { + let arg = blank_env_is_unset(Arg::new("port").env("SOUNDCRAFT_TEST_UNSET_PORT")); + assert_eq!(arg.get_env(), Some(std::ffi::OsStr::new("SOUNDCRAFT_TEST_UNSET_PORT"))); + } + + #[test] + fn conflict_is_a_clap_usage_error_naming_both_flags() { + let mut cmd = port_command().arg(Arg::new("connect").long("connect")); + let m = cmd.try_get_matches_from_mut(["t", "--port", "1", "--connect", "2"]).unwrap(); + let e = conflict(&mut cmd, &m, "port", "connect").unwrap_err(); + assert_eq!((e.kind(), e.exit_code()), (ErrorKind::ArgumentConflict, 2)); + assert!(e.to_string().starts_with("error: the argument '--port ' cannot be used with '--connect '\n\nUsage: t"), "{e}"); + let m = cmd.try_get_matches_from_mut(["t", "--port", "1"]).unwrap(); + assert!(conflict(&mut cmd, &m, "port", "connect").is_ok()); + } +} diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 352200d3..6955db33 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -5,14 +5,19 @@ //! running app's `--control` port. //! - [`mcp::Server`]: a Model Context Protocol server over stdio (newline-delimited JSON-RPC 2.0) //! exposing SoundCraft as tools and resources. +//! - [`args`]: the control and automation launch flags and their environment variables, as clap +//! types. #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +#[cfg(not(target_arch = "wasm32"))] +pub mod args; pub mod mcp; use serde_json::{Value, json}; use soundcraft_engine::Engine; use soundcraft_engine::file_access::{self, AutomationRoots}; -use std::io::{BufRead, BufReader, Write}; +use std::fmt; +use std::io::{self, BufRead, BufReader, Write}; use std::net::{TcpStream, ToSocketAddrs}; use std::sync::Arc; use std::time::Duration; @@ -79,64 +84,151 @@ impl Backend for Headless { } } +/// Where a running app's control channel listens: what `soundcraft-cli app --port`, +/// `soundcraft-cli mcp --connect`, `soundcraft --control` and SOUNDCRAFT_CONTROL_PORT give, parsed +/// once (`args::control_target`). +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ControlTarget { + /// As given: a name, an IPv4 address, or an IPv6 address in brackets (`[::1]`). `None`: + /// 127.0.0.1. + pub host: Option, + pub port: u16, +} + +impl ControlTarget { + /// Can the desktop app's control server, which listens on 127.0.0.1 only, serve this target? + /// True for no host, `127.0.0.1` and `localhost`; `[::1]` is another address. + pub fn is_loopback(&self) -> bool { + self.host.as_deref().is_none_or(|host| ["127.0.0.1", "localhost"].contains(&host)) + } +} + +/// `host:port`, as a socket address is written. +impl fmt::Display for ControlTarget { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}:{}", self.host.as_deref().unwrap_or("127.0.0.1"), self.port) + } +} + +/// Why a [`Remote`] request failed. +#[derive(Debug, thiserror::Error)] +pub enum RemoteError { + /// The host name didn't resolve. + #[error("{addr}: {source}")] + Resolve { addr: String, source: io::Error }, + #[error("cannot reach SoundCraft at {addr}: no addresses found")] + NoAddress { addr: String }, + #[error("cannot reach SoundCraft at {addr}: {source}")] + Unreachable { addr: String, source: io::Error }, + /// Reading or writing the connection failed. + #[error(transparent)] + Io(#[from] io::Error), + /// The app closed the connection before it replied. + #[error("connection closed")] + Closed, + /// The reply isn't JSON. + #[error(transparent)] + Reply(#[from] serde_json::Error), + /// The app's reply to a request it refused. + #[error("{0}")] + App(String), + /// The connection was lost after a request that changes state went out: it isn't repeated, + /// since the app may already have executed it. + #[error("{source}; `{method}` may or may not have been executed, check the session before repeating it")] + Unconfirmed { method: String, source: Box }, +} + +impl RemoteError { + /// The connection went away, so a fresh one may succeed. + fn is_lost_connection(&self) -> bool { + match self { + RemoteError::Closed => true, + RemoteError::Io(e) => matches!(e.kind(), io::ErrorKind::BrokenPipe | io::ErrorKind::ConnectionReset | io::ErrorKind::ConnectionAborted), + _ => false, + } + } +} + +type Connection = (TcpStream, BufReader); + /// A TCP client for a running app's control channel. pub struct Remote { + /// The target as `host:port`, for connecting and for messages. addr: String, - conn: Option<(TcpStream, BufReader)>, + conn: Option, next_id: u64, } impl Remote { - /// `target` is a port (`7979`) or `host:port`. - pub fn new(target: &str) -> Self { - let addr = if target.contains(':') { target.to_string() } else { format!("127.0.0.1:{target}") }; - Remote { addr, conn: None, next_id: 1 } + /// A client for the app at `target`; it connects on the first request. + pub fn new(target: &ControlTarget) -> Self { + Remote { addr: target.to_string(), conn: None, next_id: 1 } } - fn connect(&mut self) -> Result<(), String> { - if self.conn.is_some() { - return Ok(()); + /// The open connection, connecting first when there is none. + fn connection(&mut self) -> Result<&mut Connection, RemoteError> { + match self.conn { + Some(ref mut conn) => Ok(conn), + None => Ok(self.conn.insert(connect(&self.addr)?)), } - let socks: Vec<_> = self.addr.to_socket_addrs().map_err(|e| format!("{}: {e}", self.addr))?.collect(); - let mut last = "no addresses found".to_string(); - let mut stream = None; - for sock in &socks { - match TcpStream::connect_timeout(sock, Duration::from_millis(800)) { - Ok(s) => { - stream = Some(s); - break; - } - Err(e) => last = e.to_string(), - } - } - let s = stream.ok_or_else(|| format!("cannot reach SoundCraft at {}: {last}", self.addr))?; - s.set_read_timeout(Some(Duration::from_secs(120))).map_err(|e| e.to_string())?; - let r = s.try_clone().map_err(|e| e.to_string())?; - self.conn = Some((s, BufReader::new(r))); - Ok(()) } /// One request/reply exchange. On failure the flag says whether the request line was already /// written to the socket (so the app may have executed it). - fn roundtrip(&mut self, method: &str, params: &Value) -> Result { - self.connect().map_err(|e| (e, false))?; + fn roundtrip(&mut self, method: &str, params: &Value) -> Result { let id = self.next_id; self.next_id += 1; let line = json!({"id": id, "method": method, "params": params}).to_string(); - let (w, r) = self.conn.as_mut().ok_or_else(|| ("not connected".to_string(), false))?; - writeln!(w, "{line}").map_err(|e| (e.to_string(), false))?; + let (w, r) = self.connection().map_err(|e| (e, false))?; + writeln!(w, "{line}").map_err(|e| (e.into(), false))?; let mut buf = String::new(); - r.read_line(&mut buf).map_err(|e| (e.to_string(), true))?; + r.read_line(&mut buf).map_err(|e| (e.into(), true))?; if buf.is_empty() { - return Err(("connection closed".into(), true)); + return Err((RemoteError::Closed, true)); } - let v: Value = serde_json::from_str(&buf).map_err(|e| (e.to_string(), true))?; + let v: Value = serde_json::from_str(&buf).map_err(|e| (e.into(), true))?; if v.get("ok").and_then(Value::as_bool).unwrap_or(false) { Ok(v.get("result").cloned().unwrap_or(Value::Null)) } else { - Err((v.get("error").and_then(Value::as_str).unwrap_or("error").to_string(), true)) + Err((RemoteError::App(v.get("error").and_then(Value::as_str).unwrap_or("error").to_string()), true)) } } + + /// Calls `method`. A request lost with its connection is sent once more on a fresh one when + /// it only reads state. + pub fn request(&mut self, method: &str, params: Value) -> Result { + match self.roundtrip(method, ¶ms) { + Ok(v) => Ok(v), + Err((e, sent)) if e.is_lost_connection() => { + self.conn = None; + if sent && !is_read_only(method) { + return Err(RemoteError::Unconfirmed { method: method.to_owned(), source: Box::new(e) }); + } + self.roundtrip(method, ¶ms).map_err(|(e, _)| e) + } + Err((e, _)) => Err(e), + } + } +} + +/// Connects to `addr` (`host:port`), trying each address it resolves to. +fn connect(addr: &str) -> Result { + let socks = addr.to_socket_addrs().map_err(|source| RemoteError::Resolve { addr: addr.to_owned(), source })?; + let mut last = None; + for sock in socks { + match TcpStream::connect_timeout(&sock, Duration::from_millis(800)) { + Ok(s) => { + s.set_read_timeout(Some(Duration::from_secs(120)))?; + let r = s.try_clone()?; + return Ok((s, BufReader::new(r))); + } + Err(e) => last = Some(e), + } + } + Err(match last { + Some(source) => RemoteError::Unreachable { addr: addr.to_owned(), source }, + None => RemoteError::NoAddress { addr: addr.to_owned() }, + }) } /// Methods that only read state, so repeating them after a lost reply is harmless. @@ -146,19 +238,7 @@ fn is_read_only(method: &str) -> bool { impl Backend for Remote { fn call(&mut self, method: &str, params: Value) -> Result { - match self.roundtrip(method, ¶ms) { - Ok(v) => Ok(v), - Err((e, sent)) if e.contains("closed") || e.contains("Broken pipe") || e.contains("reset") => { - self.conn = None; - if sent && !is_read_only(method) { - // The app may already have executed the request; repeating it could apply it twice. - return Err(format!("{e}; `{method}` may or may not have been executed, check the session before repeating it")); - } - // Retry once on a fresh connection. - self.roundtrip(method, ¶ms).map_err(|(e, _)| e) - } - Err((e, _)) => Err(e), - } + self.request(method, params).map_err(|e| e.to_string()) } fn has_ui(&self) -> bool { true @@ -185,23 +265,25 @@ mod tests { #[test] fn remote_reports_unreachable() { - let mut r = Remote::new("127.0.0.1:1"); - assert!(r.call("session.inspect", json!({})).is_err()); + let mut r = Remote::new(&ControlTarget { host: Some("127.0.0.1".into()), port: 1 }); + let e = r.request("session.inspect", json!({})).unwrap_err(); + assert!(matches!(e, RemoteError::Unreachable { .. }), "{e:?}"); + assert!(e.to_string().starts_with("cannot reach SoundCraft at 127.0.0.1:1: "), "{e}"); } #[test] fn remote_resolves_hostnames() { let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let port = listener.local_addr().unwrap().port(); - let mut r = Remote::new(&format!("localhost:{port}")); - assert_eq!(r.connect(), Ok(())); + let mut r = Remote::new(&ControlTarget { host: Some("localhost".into()), port }); + assert!(r.connection().is_ok()); } /// Fake app: accepts one connection per entry in `replies`, counts the request line it reads, /// then either drops the connection (`false`) or answers ok (`true`). - fn fake_app(replies: Vec) -> (String, Arc) { + fn fake_app(replies: Vec) -> (ControlTarget, Arc) { let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap().to_string(); + let addr = ControlTarget { host: None, port: listener.local_addr().unwrap().port() }; let seen = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let counter = seen.clone(); std::thread::spawn(move || { diff --git a/crates/automation/src/mcp.rs b/crates/automation/src/mcp.rs index 886b25d7..e4ac191a 100644 --- a/crates/automation/src/mcp.rs +++ b/crates/automation/src/mcp.rs @@ -2,6 +2,7 @@ use crate::Backend; use serde_json::{Value, json}; +use soundcraft_audio_io::FileFormat; use std::io::{BufRead, Write}; pub const PROTOCOL_VERSION: &str = "2025-06-18"; @@ -70,7 +71,7 @@ pub fn tool_definitions() -> Vec { tool( "bounce_mix", "Render the mix to an audio file. Returns peak dBFS and integrated LUFS.", - json!({"path": {"type": "string"}, "format": {"type": "string", "enum": ["wav", "aiff", "flac"]}, "bit_depth": {"type": "string"}, "start": {}, "end": {}, "normalize": {"type": "boolean"}}), + json!({"path": {"type": "string"}, "format": {"type": "string", "enum": FileFormat::writable_extensions()}, "bit_depth": {"type": "string"}, "start": {}, "end": {}, "normalize": {"type": "boolean"}}), &["path"], ), tool("parity", "Feature-parity report against the incumbent's menu catalog.", json!({}), &[]), diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index 7c6c7c54..d725efec 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -310,7 +310,7 @@ pub fn recover(e: &mut Engine, left: &Leftover) -> Result> { let mut r = Engine::default(); let session = left.dir.join(SESSION); let links = left.origin.as_deref().map_or(session.as_path(), Path::new); - let mut missing = open_session_linked(&mut r, &session.to_string_lossy(), links)?; + let mut missing = open_session_linked(&mut r, &session, links)?; let mut s = r.session().clone(); let unfinished = unfinished_takes(left, &s, &mut missing); if let Some(home) = left.origin.as_deref().and_then(|o| Path::new(o).parent()) { @@ -535,7 +535,7 @@ mod tests { w.flush().unwrap(); let file = std::fs::read_dir(&root).unwrap().flatten().map(|d| d.path().join(SESSION)).collect::>(); let mut r = Engine::default(); - open_session(&mut r, &file[0].to_string_lossy()).unwrap(); + open_session(&mut r, &file[0]).unwrap(); assert_eq!((file.len(), r.session().sources.len()), (1, 5)); w.close(false); w.flush().unwrap(); diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 2d75b33c..f221014c 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -92,11 +92,9 @@ pub fn specs() -> Vec { let dir = str_param(p, "dir").ok_or_else(|| bad("file.bounce_stems", "`dir` required"))?.to_string(); let r = range_param(e, "file.bounce_stems", p)?; let r = if r.is_empty() { soundcraft_time::Range::new(0, e.session().content_end().max(1)) } else { r }; - let format = match str_param(p, "format") { - Some("aiff") | Some("aif") => soundcraft_audio_io::FileFormat::Aiff, - Some("flac") => soundcraft_audio_io::FileFormat::Flac, - _ => soundcraft_audio_io::FileFormat::Wav, - }; + let format = str_param(p, "format") + .and_then(soundcraft_audio_io::FileFormat::from_write_extension) + .unwrap_or(soundcraft_audio_io::FileFormat::Wav); let bit_depth = match p.get("bit_depth").map(|b| b.to_string().trim_matches('"').to_string()).as_deref() { Some("16") => soundcraft_audio_io::BitDepth::Int16, Some("32") | Some("32f") => soundcraft_audio_io::BitDepth::Float32, @@ -313,8 +311,9 @@ pub fn bounce_job(e: &Engine, p: &Value) -> Result { let r = if r.is_empty() { soundcraft_time::Range::new(0, e.session().content_end().max(1)) } else { r }; let ext = std::path::Path::new(&path).extension().and_then(std::ffi::OsStr::to_str).unwrap_or("wav").to_ascii_lowercase(); let format_name = str_param(p, "format").unwrap_or(ext.as_str()); - let format = soundcraft_audio_io::encode_format_for(format_name).ok_or_else(|| { - bad("file.bounce_mix", format!("cannot write `{format_name}` files (supported: {})", soundcraft_audio_io::ENCODE_EXTENSIONS)) + let format = soundcraft_audio_io::FileFormat::from_write_extension(format_name).ok_or_else(|| { + let supported = soundcraft_audio_io::FileFormat::writable_extensions().join(", "); + bad("file.bounce_mix", format!("cannot write `{format_name}` files (supported: {supported})")) })?; let bit_depth = match p.get("bit_depth").map(|b| b.to_string().trim_matches('"').to_string()).as_deref() { Some("16") => soundcraft_audio_io::BitDepth::Int16, diff --git a/crates/engine/src/cmd/sampler.rs b/crates/engine/src/cmd/sampler.rs index f4634e3d..03799b02 100644 --- a/crates/engine/src/cmd/sampler.rs +++ b/crates/engine/src/cmd/sampler.rs @@ -232,7 +232,8 @@ mod tests { let buf = AudioBuffer { sample_rate: 96_000, channels: vec![tone.clone(), tone] }; let mut e = Engine::default(); let t = instrument_track(&mut e); - for (ext, format) in [("flac", FileFormat::Flac), ("aif", FileFormat::Aiff)] { + for format in [FileFormat::Flac, FileFormat::Aiff] { + let ext = format.default_extension(); let path = d.join(format!("tone.{ext}")); let opts = EncodeOptions { format, bit_depth: BitDepth::Int24, dither: false, bwf: None }; std::fs::write(&path, soundcraft_audio_io::encode(&buf, &opts).unwrap()).unwrap(); diff --git a/crates/engine/src/cmd/video.rs b/crates/engine/src/cmd/video.rs index b3a6402a..14476653 100644 --- a/crates/engine/src/cmd/video.rs +++ b/crates/engine/src/cmd/video.rs @@ -165,7 +165,7 @@ mod tests { let sess = dir.join("s.scraft"); e.execute("session.save_as", &json!({"path": sess.to_string_lossy()})).unwrap(); let mut e2 = Engine::default(); - let missing = crate::io::open_session(&mut e2, &sess.to_string_lossy()).unwrap(); + let missing = crate::io::open_session(&mut e2, &sess).unwrap(); assert!(missing.is_empty(), "{missing:?}"); assert_eq!(e2.session().tracks[0].clips()[0], clip); assert_eq!(e2.session().videos, e.session().videos); @@ -173,12 +173,12 @@ mod tests { std::fs::create_dir_all(dir.join("Video Files")).unwrap(); std::fs::rename(&movie, dir.join("Video Files").join("picture.mp4")).unwrap(); let mut e3 = Engine::default(); - assert!(crate::io::open_session(&mut e3, &sess.to_string_lossy()).unwrap().is_empty()); + assert!(crate::io::open_session(&mut e3, &sess).unwrap().is_empty()); assert!(e3.session().videos[0].path.contains("Video Files")); // Movie gone: the session still opens and reports it. std::fs::remove_file(dir.join("Video Files").join("picture.mp4")).unwrap(); let mut e4 = Engine::default(); - let missing = crate::io::open_session(&mut e4, &sess.to_string_lossy()).unwrap(); + let missing = crate::io::open_session(&mut e4, &sess).unwrap(); assert!(missing.iter().any(|m| m.contains("picture.mp4")), "{missing:?}"); assert!(e4.messages.iter().any(|m| m.contains("missing media"))); assert_eq!(e4.session().tracks[0].clips().len(), 1); diff --git a/crates/engine/src/file_access.rs b/crates/engine/src/file_access.rs index 98c11a9c..73e92c66 100644 --- a/crates/engine/src/file_access.rs +++ b/crates/engine/src/file_access.rs @@ -48,16 +48,28 @@ struct Root { shown: PathBuf, } +/// A root [`AutomationRoots::new`] can't open: missing, unreadable, or not a folder. +#[derive(Debug, thiserror::Error)] +#[error("cannot open the automation {} root `{}`: {source}", kind(.access), .path.display())] +pub struct RootError { + access: Access, + path: PathBuf, + source: std::io::Error, +} + +fn kind(access: &Access) -> &'static str { + match access { + Access::Read => "read", + Access::Write => "write", + } +} + impl Root { /// The folder at `path`, which must exist. - fn open(path: &Path, access: Access) -> Result { - let kind = match access { - Access::Read => "read", - Access::Write => "write", - }; + fn open(path: &Path, access: Access) -> Result { let resolved = std::fs::canonicalize(path) .and_then(|p| if p.is_dir() { Ok(p) } else { Err(std::io::ErrorKind::NotADirectory.into()) }) - .map_err(|e| format!("cannot open the automation {kind} root `{}`: {e}", path.display()))?; + .map_err(|source| RootError { access, path: path.to_owned(), source })?; Ok(Self { shown: plain(resolved.clone()), resolved }) } } @@ -65,11 +77,11 @@ impl Root { impl AutomationRoots { /// The roots `read` and `write`, which must be existing folders (one given twice counts once); /// `None` when neither kind has any (nothing is confined then). - pub fn new(read: &[&Path], write: &[&Path]) -> Result>, String> { + pub fn new(read: &[&Path], write: &[&Path]) -> Result>, RootError> { if read.is_empty() && write.is_empty() { return Ok(None); } - let open = |paths: &[&Path], access: Access| -> Result, String> { + let open = |paths: &[&Path], access: Access| -> Result, RootError> { let mut roots: Vec = Vec::with_capacity(paths.len()); for path in paths { let root = Root::open(path, access)?; @@ -354,15 +366,15 @@ mod tests { #[test] fn a_missing_root_is_an_error_and_none_is_no_confinement() { let (base, _) = folders("missing"); - let e = AutomationRoots::new(&[&base.join("nope")], &[]).unwrap_err(); + let e = AutomationRoots::new(&[&base.join("nope")], &[]).unwrap_err().to_string(); assert!(e.starts_with("cannot open the automation read root"), "{e}"); - let e = AutomationRoots::new(&[], &[&base.join("in/a.scraft")]).unwrap_err(); + let e = AutomationRoots::new(&[], &[&base.join("in/a.scraft")]).unwrap_err().to_string(); assert!( e.starts_with("cannot open the automation write root") && e.ends_with(&std::io::Error::from(std::io::ErrorKind::NotADirectory).to_string()), "{e}" ); - assert!(AutomationRoots::new(&[Path::new("")], &[]).unwrap_err().starts_with("cannot open the automation read root ``")); + assert!(AutomationRoots::new(&[Path::new("")], &[]).unwrap_err().to_string().starts_with("cannot open the automation read root ``")); assert!(AutomationRoots::new(&[], &[]).unwrap().is_none()); // One root alone grants only its own access. let read_only = AutomationRoots::new(&[&base.join("in")], &[]).unwrap().unwrap(); @@ -440,7 +452,7 @@ mod tests { let twice = AutomationRoots::new(&[&base.join("in"), &base.join("in/sub/..")], &[]).unwrap().unwrap(); assert!(!twice.describe().contains(", ") && twice.describe().ends_with("; write none"), "{}", twice.describe()); // A missing folder among several is an error. - let e = AutomationRoots::new(&[&base.join("in"), &base.join("nope")], &[]).unwrap_err(); + let e = AutomationRoots::new(&[&base.join("in"), &base.join("nope")], &[]).unwrap_err().to_string(); assert!(e.starts_with("cannot open the automation read root"), "{e}"); let _ = std::fs::remove_dir_all(base); } diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 76a33c7d..c457dcf2 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -424,11 +424,7 @@ pub fn bounce_stems_with(e: &Engine, dir: &str, r: Range, opts: &EncodeOptions, return Err(EngineError::BadParams("file.bounce_stems".into(), "bounces are limited to 4 hours".into())); } make_dir(dir)?; - let ext = match opts.format { - FileFormat::Aiff => "aif", - FileFormat::Flac => "flac", - _ => "wav", - }; + let ext = opts.format.default_extension(); let mut out = Vec::new(); let stems: Vec<(TrackId, String)> = s .tracks diff --git a/crates/engine/src/io/session.rs b/crates/engine/src/io/session.rs index f92b8624..0c934f58 100644 --- a/crates/engine/src/io/session.rs +++ b/crates/engine/src/io/session.rs @@ -34,16 +34,17 @@ pub fn save_session_copy(e: &Engine, path: &str) -> Result { } /// Open a `.scraft` session and decode its audio files (missing files are reported, not fatal). -pub fn open_session(e: &mut Engine, path: &str) -> Result> { - open_session_linked(e, path, Path::new(path)) +pub fn open_session(e: &mut Engine, path: impl AsRef) -> Result> { + let path = path.as_ref(); + open_session_linked(e, path, path) } /// [`open_session`], following the media paths in it as if they were stored in the session file /// `links` (crash recovery: the session written beside the autosave, its paths from the original). -pub(crate) fn open_session_linked(e: &mut Engine, path: &str, links: &Path) -> Result> { +pub(crate) fn open_session_linked(e: &mut Engine, path: &Path, links: &Path) -> Result> { let text = read_text(path)?; let mut s = Session::from_json(&text).map_err(|err| EngineError::Io(err.to_string()))?; - let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); + let dir = path.parent().map(Path::to_path_buf).unwrap_or_default(); let mut missing = Vec::new(); let mut files = DecodedFiles::new(); for src in &s.sources { @@ -57,7 +58,9 @@ pub(crate) fn open_session_linked(e: &mut Engine, path: &str, links: &Path) -> R missing.extend(relink_videos(&mut s, links)); s.folder = Some(dir); e.replace_session(s); - e.path = Some(path.to_string()); + // Save writes back to `e.path`, which is text: a session at a name that isn't valid Unicode + // opens as if never saved, rather than saving to a name with replacement characters. + e.path = path.to_str().map(str::to_owned); for m in &missing { e.message(format!("missing media: {m}")); } @@ -192,11 +195,11 @@ mod save_tests { assert_eq!(level(&reopen(&path("B/B.scraft"))), 0.5); std::fs::remove_file(dir.join("A/Audio Files/Audio 1_02.wav")).unwrap(); let mut missing = Engine::default(); - open_session(&mut missing, &path("A/A.scraft")).unwrap(); + open_session(&mut missing, path("A/A.scraft")).unwrap(); save_session(&mut missing, &path("B/Moved.scraft")).unwrap(); let mut r = Engine::default(); assert_eq!( - open_session(&mut r, &path("B/Moved.scraft")).unwrap(), + open_session(&mut r, path("B/Moved.scraft")).unwrap(), vec![dir.join("A").join("Audio Files").join("Audio 1_02.wav").display().to_string()] ); let _ = std::fs::remove_dir_all(&dir); @@ -222,7 +225,7 @@ mod save_tests { Engine::default().execute("session.save_as", &json!({"path": dir.join("New/New.scraft")})).unwrap(); std::fs::copy(&path, dir.join("Song")).unwrap(); let mut bare = Engine::default(); - open_session(&mut bare, &dir.join("Song").to_string_lossy()).unwrap(); + open_session(&mut bare, dir.join("Song")).unwrap(); bare.execute("session.save", &json!({})).unwrap(); second.execute("session.save_as", &json!({"path": path, "replace": true})).unwrap(); let r = reopen(&path); diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index c3289263..b0807213 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -16,6 +16,12 @@ pub(crate) use playback::automation_step_due; pub(crate) use plugins::{editor_changes, editor_states_due, failed_plugin_status}; impl SoundApp { + /// Whether fonts installed from the system are used besides the bundled ones (on by default). + /// Takes effect only before the first frame installs the fonts. + pub fn set_system_fonts(&mut self, on: bool) { + self.system_fonts = on; + } + /// Build an app around an engine and optional player, initialising UI, recording, autosave, control and /// synthetic-input state. pub fn new(engine: Engine, player: Option, services: Services) -> Self { @@ -87,6 +93,7 @@ impl SoundApp { last_rev: 0, fonts_ready: false, fonts_installed: false, + system_fonts: true, theme_applied: None, docking_context: None, sim: None, @@ -130,7 +137,7 @@ impl SoundApp { if self.fonts_installed { self.fonts_ready = true; } else { - fonts::install(ctx); + fonts::install(ctx, self.system_fonts); self.fonts_installed = true; ctx.request_repaint(); } diff --git a/crates/ui-egui/src/app/input.rs b/crates/ui-egui/src/app/input.rs index 73ba2a5d..cafbed86 100644 --- a/crates/ui-egui/src/app/input.rs +++ b/crates/ui-egui/src/app/input.rs @@ -139,18 +139,22 @@ impl SoundApp { } let mut claimed = false; for (file, pos) in std::mem::take(&mut self.pending_drops) { - let path = file.path().to_string_lossy().into_owned(); // As before, one dropped audio file goes to the sampler under the pointer; the rest // are left to the shell. - let claim = !claimed && sampler_window::is_audio_name(&path); + let claim = !claimed && sampler_window::is_audio_name(file.path()); let Some(track) = self.drop_track(pos).filter(|_| claim) else { self.host_drops.push(file); continue; }; claimed = true; if file.path().is_absolute() { - let r = self.run("mix.sampler_load", json!({"track": track.0, "path": path})); - self.sample_loaded(track, &path, r); + match crate::dialogs::unicode(file.path()) { + Ok(path) => { + let r = self.run("mix.sampler_load", json!({"track": track.0, "path": path})); + self.sample_loaded(track, path, r); + } + Err(e) => self.ui.status = e.to_string(), + } } else { self.sample_reads.push(sampler_window::SampleRead { track, file }); } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 73babcec..17543691 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -4,6 +4,7 @@ use crate::SoundApp; use crate::theme::{Tokens, bold}; use egui::{Align2, vec2}; use serde_json::{Value, json}; +use soundcraft_audio_io::FileFormat; use soundcraft_engine::Engine; use soundcraft_engine::io::StripNames; use soundcraft_model::{ClipId, GroupId, Session, SourceId, TrackId}; @@ -17,8 +18,8 @@ mod strip_silence; #[cfg(test)] use audiosuite::SPEED_RAMP_CHOICES; use audiosuite::audiosuite_window; +pub use files::{NotUnicode, ask_to_save, open_path, open_session_bytes, run_or_ask, takes_path, unicode}; use files::{OPEN_PENDING, default_dir, open_pending_session, path_dialog}; -pub use files::{ask_to_save, open_session_bytes, run_or_ask, takes_path}; use strip_silence::{AUTO_NAME_LABELS, STRIP_NAME_LABELS, StripRectangles, auto_rename_action, strip_name_fields, strip_silence_action}; pub use strip_silence::{StripRects, strip_silence_rectangles}; @@ -31,7 +32,7 @@ pub mod spot; pub enum Dialog { NewTracks { count: u32, format: String, kind: String, timebase: String, name: String }, NewSession { name: String, sample_rate: u32, demo: bool }, - Bounce { path: String, format: String, bit_depth: String, normalize: bool, dither: bool }, + Bounce { path: String, format: FileFormat, bit_depth: String, normalize: bool, dither: bool }, RenameTrack { id: TrackId, name: String }, RenameClip { id: ClipId, name: String }, RenameMarker { id: soundcraft_model::MarkerId, name: String }, @@ -178,7 +179,7 @@ impl Dialogs { .map_or(home.clone(), |d| d.display().to_string()); Dialog::Bounce { path: format!("{dir}/{name} Bounce.wav"), - format: "wav".into(), + format: FileFormat::Wav, bit_depth: "24".into(), normalize: false, dither: false, @@ -391,21 +392,23 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { crate::widgets::named_text_field(ui.add(egui::TextEdit::singleline(path).desired_width(420.0)), "File"); if let Some(pick) = &app.services.pick_save && ui.button("Browse…").clicked() - && let Some(p) = pick("file.bounce_mix", path) + && let Some(p) = pick("file.bounce_mix", std::path::Path::new(path.as_str())) { - *path = p; - match std::path::Path::new(path.as_str()).extension().map(|x| x.to_string_lossy().to_lowercase()).as_deref() { - Some("wav") => *format = "wav".into(), - Some("aif" | "aiff") => *format = "aiff".into(), - Some("flac") => *format = "flac".into(), - _ => {} + match unicode(&p) { + Ok(text) => { + *path = text.to_owned(); + if let Some(picked) = p.extension().and_then(std::ffi::OsStr::to_str).and_then(FileFormat::from_write_extension) { + *format = picked; + } + } + Err(e) => app.ui.status = e.to_string(), } } }); ui.horizontal(|ui| { - craft_ui::ComboBox::from_label("Format").selected_text(format.to_uppercase()).show_ui(ui, |ui| { - for f in ["wav", "aiff", "flac"] { - ui.selectable_value(format, f.to_string(), f.to_uppercase()); + craft_ui::ComboBox::from_label("Format").selected_text(format.name()).show_ui(ui, |ui| { + for &f in FileFormat::WRITE { + ui.selectable_value(format, f, f.name()); } }); craft_ui::ComboBox::from_label("Bit Depth").selected_text(format!("{bit_depth}-bit")).show_ui(ui, |ui| { @@ -426,10 +429,10 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { .color(Tokens::current().text_dim), ); if buttons(ui, "Bounce", enter) { - let p = std::path::Path::new(path.as_str()).with_extension(if format == "aiff" { "aif" } else { format.as_str() }); + let p = std::path::Path::new(path.as_str()).with_extension(format.default_extension()); crate::bounce::start( app, - &json!({"path": p.to_string_lossy(), "format": format, "bit_depth": bit_depth, "normalize": normalize, "dither": *dither && bit_depth.as_str() != "32f"}), + &json!({"path": p.to_string_lossy(), "format": format.default_extension(), "bit_depth": bit_depth, "normalize": normalize, "dither": *dither && bit_depth.as_str() != "32f"}), ); keep = false; } @@ -461,6 +464,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { } Dialog::PathPrompt { cmd, path, key, .. } => { crate::widgets::named_text_field(ui.add(egui::TextEdit::singleline(path).desired_width(420.0)), "Path"); + let mut picked = None; if let Some(pick) = &app.services.pick_open && cmd != "session.save_as" && cmd != "session.save_copy" @@ -468,21 +472,26 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { && ui.button("Browse…").clicked() && let Some(p) = pick(cmd, &[]) { - *path = p; + picked = Some(p); } if let Some(pick) = &app.services.pick_folder && key == "dir" && ui.button("Browse…").clicked() - && let Some(p) = pick(path) + && let Some(p) = pick(std::path::Path::new(path.as_str())) { - *path = p; + picked = Some(p); } if let Some(pick) = &app.services.pick_save && (cmd == "session.save_as" || cmd == "session.save_copy") && ui.button("Browse…").clicked() - && let Some(p) = pick(cmd, path) + && let Some(p) = pick(cmd, std::path::Path::new(path.as_str())) { - *path = p; + picked = Some(p); + } + match picked.as_deref().map(unicode) { + Some(Ok(text)) => *path = text.to_owned(), + Some(Err(e)) => app.ui.status = e.to_string(), + None => {} } let file = soundcraft_engine::io::session_file(path); let replaces = soundcraft_engine::io::replaces(&app.engine, cmd, &file); @@ -860,6 +869,21 @@ fn loop_length(s: &Session, text: &str, format: soundcraft_time::TimeFormat, sta mod tests { use super::*; + /// A name that is not valid Unicode is not opened under a lossy rendering (another file): the + /// status line names its bytes. + #[cfg(unix)] + #[test] + fn non_unicode_paths_are_reported_not_mangled() { + use std::os::unix::ffi::OsStrExt; + assert_eq!(unicode(std::path::Path::new("/a/café.wav")), Ok("/a/café.wav")); + let latin1 = std::path::Path::new(std::ffi::OsStr::from_bytes(b"/a/caf\xe9.scraft")); + assert_eq!(unicode(latin1), Err(NotUnicode(latin1.to_path_buf()))); + let mut app = SoundApp::new(Engine::default(), None, crate::Services::default()); + open_path(&mut app, latin1); + assert_eq!(app.ui.status, r#""/a/caf\xE9.scraft" is not valid Unicode"#); + assert!(app.engine.path.is_none() && app.dialogs.open.is_none()); + } + #[test] fn file_commands_get_path_prompts_with_sensible_names() { let e = soundcraft_engine::demo::demo_engine(); diff --git a/crates/ui-egui/src/dialogs/files.rs b/crates/ui-egui/src/dialogs/files.rs index 4906f27a..a2d8d0a0 100644 --- a/crates/ui-egui/src/dialogs/files.rs +++ b/crates/ui-egui/src/dialogs/files.rs @@ -26,6 +26,51 @@ pub fn run_or_ask(app: &mut SoundApp, id: &str, params: Value) -> Result) -> std::fmt::Result { + write!(f, "{:?} is not valid Unicode", self.0) + } +} + +impl std::error::Error for NotUnicode {} + +/// `path` as the text the engine's commands take. Convert only where a command gets the path: +/// a lossy rendering would name another file. +pub fn unicode(path: &std::path::Path) -> Result<&str, NotUnicode> { + path.to_str().ok_or_else(|| NotUnicode(path.to_path_buf())) +} + +/// Opens a dropped, picked or command-line file by its extension: a session opens (asking to save +/// first), MIDI imports, and anything else imports as audio, decoded in the background so a long +/// file doesn't freeze the window (#597). A name that is not valid Unicode goes on the status line. +pub fn open_path(app: &mut SoundApp, path: &std::path::Path) { + let p = match unicode(path) { + Ok(p) => p, + Err(e) => { + log::warn!("{e}"); + app.ui.status = e.to_string(); + return; + } + }; + let lower = p.to_ascii_lowercase(); + let r = if lower.ends_with(".scraft") { + run_or_ask(app, "session.open", json!({"path": p})) + } else if lower.ends_with(".mid") || lower.ends_with(".midi") || lower.ends_with(".smf") { + app.run("file.import_midi", json!({"path": p})) + } else { + let at = app.engine.session().edit.selection.start; + crate::import::start(app, json!({"path": p, "at": at})); + Ok(Value::Null) + }; + if let Err(e) = r { + log::warn!("{p}: {e}"); + } +} + /// What `SaveChanges` carries on with to open the session the browser read. pub(super) const OPEN_PENDING: &str = "ui.open_pending_session"; diff --git a/crates/ui-egui/src/edit_window/tests.rs b/crates/ui-egui/src/edit_window/tests.rs index cd3dc3da..f7f94bbe 100644 --- a/crates/ui-egui/src/edit_window/tests.rs +++ b/crates/ui-egui/src/edit_window/tests.rs @@ -6,7 +6,7 @@ use egui_kittest::{Harness, kittest::Queryable}; /// they edit, not the short value text ("Read", "Waveform") painted inside them. #[test] fn track_header_selectors_have_accessible_names() { - let mut h = Harness::builder().with_context_setup(crate::fonts::install).build_ui(|ui| { + let mut h = Harness::builder().with_context_setup(|ctx| crate::fonts::install(ctx, true)).build_ui(|ui| { let view = Rect::from_min_size(Pos2::ZERO, vec2(60.0, 17.0)); super::header_selector(ui, view, egui::Id::unique("test-view"), "Waveform", Color32::WHITE, true, "Track view"); let mode = Rect::from_min_size(pos2(0.0, 20.0), vec2(60.0, 17.0)); diff --git a/crates/ui-egui/src/fonts.rs b/crates/ui-egui/src/fonts.rs index 3f9ce63a..514c10a1 100644 --- a/crates/ui-egui/src/fonts.rs +++ b/crates/ui-egui/src/fonts.rs @@ -80,8 +80,8 @@ fn load_from_fontconfig(pattern: &str, lang: Option<&str>) -> Option { } #[cfg(not(target_arch = "wasm32"))] -fn load(cands: &[(&str, u32)], pattern: &str, lang: Option<&str>) -> Option { - if std::env::var_os("SOUNDCRAFT_NO_SYSTEM_FONTS").is_some() { +fn load(cands: &[(&str, u32)], pattern: &str, lang: Option<&str>, system_fonts: bool) -> Option { + if !system_fonts { return None; } #[cfg(any(target_os = "linux", target_os = "freebsd"))] @@ -104,28 +104,29 @@ fn load(cands: &[(&str, u32)], pattern: &str, lang: Option<&str>) -> Option) -> Option { +fn load(_: &[(&str, u32)], _: &str, _: Option<&str>, _: bool) -> Option { None } -pub fn definitions() -> FontDefinitions { +/// The font set: the bundled faces, plus the system ones when `system_fonts` is true. +pub fn definitions(system_fonts: bool) -> FontDefinitions { let mut fonts = FontDefinitions::default(); let base: Vec = fonts.families.get(&FontFamily::Proportional).cloned().unwrap_or_default(); let mut bold_family = base.clone(); - if let Some(reg) = load(REGULAR, "sans-serif", None) { + if let Some(reg) = load(REGULAR, "sans-serif", None, system_fonts) { fonts.font_data.insert("system-regular".into(), Arc::new(reg)); if let Some(f) = fonts.families.get_mut(&FontFamily::Proportional) { f.insert(0, "system-regular".into()); } } - if let Some(b) = load(BOLD, "sans-serif:style=Bold", None) { + if let Some(b) = load(BOLD, "sans-serif:style=Bold", None, system_fonts) { fonts.font_data.insert("system-bold".into(), Arc::new(b)); bold_family.insert(0, "system-bold".into()); } else if fonts.font_data.contains_key("system-regular") { bold_family.insert(0, "system-regular".into()); } fonts.families.insert(FontFamily::Name("bold".into()), bold_family); - if let Some(h) = load(HANGUL, "sans-serif:lang=ko", Some("ko")) { + if let Some(h) = load(HANGUL, "sans-serif:lang=ko", Some("ko"), system_fonts) { fonts.font_data.insert("system-hangul".into(), Arc::new(h)); for family in fonts.families.values_mut() { family.push("system-hangul".into()); @@ -134,8 +135,8 @@ pub fn definitions() -> FontDefinitions { fonts } -pub fn install(ctx: &egui::Context) { - ctx.set_fonts(definitions()); +pub fn install(ctx: &egui::Context, system_fonts: bool) { + ctx.set_fonts(definitions(system_fonts)); } #[cfg(test)] @@ -177,7 +178,7 @@ mod tests { /// Korean names draw in every family, not as boxes; skipped when the system has no Korean font. #[test] fn every_family_draws_hangul_with_a_system_korean_font() { - let defs = definitions(); + let defs = definitions(true); if !defs.font_data.contains_key("system-hangul") { eprintln!("skipping: no Korean system font"); return; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 368c10bf..a84be7e4 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -369,6 +369,8 @@ pub struct SoundApp { last_rev: u64, fonts_ready: bool, fonts_installed: bool, + /// Whether the system fonts are loaded next to the bundled ones; set before the first frame. + system_fonts: bool, /// The palette last applied to the context (light?), so visuals are set only on a change. theme_applied: Option, /// Last panel host context, used to cancel renderer-owned gestures on explicit replacement. diff --git a/crates/ui-egui/src/mix_window/sends.rs b/crates/ui-egui/src/mix_window/sends.rs index b1486087..1a9dabdc 100644 --- a/crates/ui-egui/src/mix_window/sends.rs +++ b/crates/ui-egui/src/mix_window/sends.rs @@ -426,7 +426,7 @@ mod tests { let mut h = Harness::builder().with_size(vec2(600.0, 800.0)).build_ui_state( move |ui, (app, kick): &mut (SoundApp, TrackId)| { if !ready { - crate::fonts::install(ui.ctx()); + crate::fonts::install(ui.ctx(), true); crate::theme::apply(ui.ctx(), false); ready = true; return; diff --git a/crates/ui-egui/src/panels/sampler_window.rs b/crates/ui-egui/src/panels/sampler_window.rs index a16fee9e..cc14c94c 100644 --- a/crates/ui-egui/src/panels/sampler_window.rs +++ b/crates/ui-egui/src/panels/sampler_window.rs @@ -7,22 +7,18 @@ use crate::SoundApp; use crate::theme::{Tokens, mono, regular}; use egui::{Align2, Color32, CursorIcon, Rect, Sense, Stroke, StrokeKind, Ui, pos2, vec2}; use serde_json::json; +use soundcraft_audio_io::FileFormat; use soundcraft_model::{Insert, TrackId}; -use std::ffi::OsStr; -use std::path::Path; /// The built-in Sampler's plugin id. pub const SAMPLER: &str = "sampler"; -/// File extensions a sample can come from (what the audio decoder reads). -const AUDIO_EXTS: &[&str] = &["wav", "wave", "bwf", "aif", "aiff", "aifc", "flac", "mp3", "ogg", "oga", "m4a", "aac", "mp4", "caf"]; - /// Markers on the sample strip: the parameter each one sets, and whether it needs a loop. const MARKERS: [(&str, bool); 3] = [("start", false), ("loop_start", true), ("loop_end", true)]; /// Whether a file name looks like audio the sampler can load. -pub fn is_audio_name(name: &str) -> bool { - Path::new(name).extension().and_then(OsStr::to_str).is_some_and(|x| AUDIO_EXTS.contains(&x.to_ascii_lowercase().as_str())) +pub fn is_audio_name(name: impl AsRef) -> bool { + FileFormat::from_path(name).is_some() } /// Sampler parameters that are MIDI notes. @@ -145,9 +141,14 @@ pub fn sample_strip(app: &mut SoundApp, ui: &mut Ui, tid: TrackId, ins: &Insert) let load = if can_pick { load } else { load.on_disabled_hover_text("Drop an audio file on this window") }; if load.clicked() && let Some(pick) = &app.services.pick_open - && let Some(path) = pick("mix.sampler_load", AUDIO_EXTS) + && let Some(path) = pick("mix.sampler_load", &FileFormat::readable_extensions()) { - let _ = app.run("mix.sampler_load", json!({"track": tid.0, "path": path})); + match crate::dialogs::unicode(&path) { + Ok(path) => { + let _ = app.run("mix.sampler_load", json!({"track": tid.0, "path": path})); + } + Err(e) => app.ui.status = e.to_string(), + } } if ins.sample.is_some() && ui.button("Clear").clicked() { let _ = app.run("mix.sampler_clear", json!({"track": tid.0})); diff --git a/crates/ui-egui/src/panels/windows.rs b/crates/ui-egui/src/panels/windows.rs index 9b0a085b..a0719ea2 100644 --- a/crates/ui-egui/src/panels/windows.rs +++ b/crates/ui-egui/src/panels/windows.rs @@ -4,7 +4,8 @@ use crate::SoundApp; use crate::theme::{Tokens, mono, rgb}; use egui::{Color32, Sense, vec2}; use serde_json::{Value, json}; -use soundcraft_model::{AutomationMode, ClipContent}; +use soundcraft_audio_io::FileFormat; +use soundcraft_model::{AutomationMode, ClipContent, SESSION_EXTENSION}; use soundcraft_playback::AudioConfig; use soundcraft_playback::devices::{AudioSystem, DeviceInfo}; use std::sync::Arc; @@ -334,38 +335,38 @@ fn workspace(app: &mut SoundApp, ctx: &egui::Context) { ui.label(egui::RichText::new(&dir).small()); }); ui.separator(); - let mut entries: Vec<(String, bool)> = std::fs::read_dir(&dir) + // Names stay paths: a name that is not valid Unicode is shown lossily, and opening it says so. + let mut entries: Vec<(std::path::PathBuf, String, bool)> = std::fs::read_dir(&dir) .map(|rd| { rd.flatten() .filter_map(|e| { + let path = e.path(); let name = e.file_name().to_string_lossy().into_owned(); if name.starts_with('.') { return None; } let is_dir = e.file_type().is_ok_and(|t| t.is_dir()); - let ext = std::path::Path::new(&name).extension().and_then(std::ffi::OsStr::to_str).unwrap_or("").to_ascii_lowercase(); - (is_dir || ["wav", "aif", "aiff", "flac", "mp3", "ogg", "m4a", "m4b", "caf", "mid", "midi", "scraft"].contains(&ext.as_str())) - .then_some((name, is_dir)) + let ext = path.extension().and_then(std::ffi::OsStr::to_str).unwrap_or("").to_ascii_lowercase(); + (is_dir || FileFormat::from_extension(&ext).is_some() || ["mid", "midi", SESSION_EXTENSION].contains(&ext.as_str())) + .then_some((path, name, is_dir)) }) .collect() }) .unwrap_or_default(); - entries.sort_by(|a, b| b.1.cmp(&a.1).then(a.0.to_lowercase().cmp(&b.0.to_lowercase()))); + entries.sort_by(|a, b| b.2.cmp(&a.2).then(a.1.to_lowercase().cmp(&b.1.to_lowercase()))); egui::ScrollArea::vertical().show(ui, |ui| { - for (name, is_dir) in entries.into_iter().take(2000) { - let path = std::path::Path::new(&dir).join(&name).to_string_lossy().into_owned(); + for (path, name, is_dir) in entries.into_iter().take(2000) { let label = if is_dir { format!("📁 {name}") } else { format!("🔊 {name}") }; let r = ui.selectable_label(false, label); if r.double_clicked() { if is_dir { - app.ui.workspace_dir = path; - } else if name.ends_with(".scraft") { - let _ = crate::dialogs::run_or_ask(app, "session.open", json!({"path": path})); - } else if name.ends_with(".mid") || name.ends_with(".midi") { - let _ = app.run("file.import_midi", json!({"path": path})); + // The folder is kept as text in the preferences. + match crate::dialogs::unicode(&path) { + Ok(p) => app.ui.workspace_dir = p.to_owned(), + Err(e) => app.ui.status = e.to_string(), + } } else { - let at = app.engine.session().edit.selection.start; - crate::import::start(app, json!({"path": path, "at": at})); + crate::dialogs::open_path(app, &path); } } } diff --git a/crates/ui-egui/src/services.rs b/crates/ui-egui/src/services.rs index 24b7cadc..e128241f 100644 --- a/crates/ui-egui/src/services.rs +++ b/crates/ui-egui/src/services.rs @@ -1,7 +1,12 @@ -/// Host services (file dialogs) injected by the app; absent in tests and on the web. +use std::path::{Path, PathBuf}; + +/// Host services (file dialogs) injected by the app; absent in tests and on the web. They return +/// paths as picked: the caller converts one to text where a command takes it ([`crate::dialogs::unicode`]). #[derive(Default)] pub struct Services { - pub pick_open: Option Option>>, - pub pick_save: Option Option>>, - pub pick_folder: Option Option>>, + pub pick_open: Option Option>>, + /// The purpose and the file to start from. + pub pick_save: Option Option>>, + /// The folder to start in. + pub pick_folder: Option Option>>, } diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 1a23ef63..e4a2acb1 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -754,7 +754,7 @@ mod selector_visual_regressions { let mut harness = egui_kittest::Harness::builder().with_size(vec2(320.0, 120.0)).with_pixels_per_point(scale).wgpu().build_ui_state( move |ui, regions: &mut serde_json::Map| { if !ready { - crate::fonts::install(ui.ctx()); + crate::fonts::install(ui.ctx(), true); crate::theme::apply(ui.ctx(), false); ready = true; return; @@ -807,7 +807,7 @@ mod selector_visual_regressions { for mode in soundcraft_model::AutomationMode::ALL { for scale in [1.0, 1.5, 2.0] { let ctx = egui::Context::default(); - crate::fonts::install(&ctx); + crate::fonts::install(&ctx, true); crate::theme::apply(&ctx, false); let mut app = mixer_app(narrow, mode); let mut output = ctx.run_ui(scaled_input(vec2(320.0, 640.0), scale), |ui| crate::mix_window::show(&mut app, ui)); @@ -856,7 +856,7 @@ mod selector_visual_regressions { egui_kittest::Harness::builder().with_size(vec2(320.0, 640.0)).with_pixels_per_point(scale).wgpu().build_ui_state( move |ui, app: &mut crate::SoundApp| { if !ready { - crate::fonts::install(ui.ctx()); + crate::fonts::install(ui.ctx(), true); crate::theme::apply(ui.ctx(), false); ready = true; return; @@ -902,7 +902,7 @@ mod selector_visual_regressions { for label in ["auto read", "no group", "A very long mixer group name"] { for scale in [1.0, 1.5, 2.0] { let ctx = egui::Context::default(); - crate::fonts::install(&ctx); + crate::fonts::install(&ctx, true); crate::theme::apply(&ctx, false); let mut rect = Rect::NOTHING; let mut output = ctx.run_ui(scaled_input(vec2(320.0, 120.0), scale), |ui| { diff --git a/crates/ui-egui/tests/it/bounce_dialog.rs b/crates/ui-egui/tests/it/bounce_dialog.rs index e3a5499c..a712f4c3 100644 --- a/crates/ui-egui/tests/it/bounce_dialog.rs +++ b/crates/ui-egui/tests/it/bounce_dialog.rs @@ -1,16 +1,18 @@ //! Bounce Mix starts in the session's folder and has a Browse button for the file (#186). use egui::vec2; use egui_kittest::{Harness, kittest::Queryable}; +use soundcraft_audio_io::FileFormat; use soundcraft_ui_egui::{Services, SoundApp, dialogs::Dialog}; use std::cell::RefCell; +use std::path::{Path, PathBuf}; use std::rc::Rc; #[test] fn bounce_starts_in_the_session_folder_and_browse_picks_the_file_and_format() { let asked = Rc::new(RefCell::new(Vec::new())); let log = Rc::clone(&asked); - let pick_save: Box Option> = Box::new(move |purpose, default| { - log.borrow_mut().push((purpose.to_string(), default.to_string())); + let pick_save: Box Option> = Box::new(move |purpose, default| { + log.borrow_mut().push((purpose.to_string(), default.to_path_buf())); Some("/Mixes/Final Mix.FLAC".into()) }); let mut engine = soundcraft_engine::demo::demo_engine(); @@ -27,10 +29,10 @@ fn bounce_starts_in_the_session_folder_and_browse_picks_the_file_and_format() { h.run_steps(3); h.get_by_label("Browse…").click(); h.run_steps(3); - let default = "/Songs/Midnight Groove/Midnight Groove Bounce.wav".to_string(); + let default = PathBuf::from("/Songs/Midnight Groove/Midnight Groove Bounce.wav"); assert_eq!(*asked.borrow(), vec![("file.bounce_mix".to_string(), default)]); match &h.state().dialogs.open { - Some(Dialog::Bounce { path, format, .. }) => assert_eq!((path.as_str(), format.as_str()), ("/Mixes/Final Mix.FLAC", "flac")), + Some(Dialog::Bounce { path, format, .. }) => assert_eq!((path.as_str(), *format), ("/Mixes/Final Mix.FLAC", FileFormat::Flac)), other => panic!("{other:?}"), } } diff --git a/docs/cli.md b/docs/cli.md new file mode 100644 index 00000000..eb94ab74 --- /dev/null +++ b/docs/cli.md @@ -0,0 +1,145 @@ +# Command-line tool + +`soundcraft-cli` runs SoundCraft without a window: it converts and inspects audio, runs engine +commands on a session, serves MCP, and calls a running app. Follows +[RFC-0012](https://github.com/storytold/craft-contrib/pull/18) (command-line conventions for the +Craft apps). + +`soundcraft-cli --help` and `soundcraft-cli --help` are the source of truth for +flags, values and defaults. This page does not repeat them. + +| Subcommand | What it does | +|---|---| +| `info` | Prints an audio file's format as JSON, or a `.scraft` session's summary | +| `convert` | Converts an audio file: resample, change the bit depth, normalize | +| `run` | Runs engine commands on a new, demo or saved session; can bounce, save and inspect | +| `script` | Runs a file of engine commands, one `ID JSON` per line | +| `commands` | Lists the engine commands | +| `describe` | Prints one engine command's details as JSON | +| `app` | Calls a running app's control channel (see [the control protocol](control-protocol.md)) | +| `mcp` | Serves MCP on stdin and stdout (see [the MCP guide](mcp.md)) | +| `parity` | Prints the parity report as markdown | +| `plugins` | Lists the built-in plugins | + +```sh +soundcraft-cli --version # also -V; there is no `version` subcommand +soundcraft-cli help convert # the same as `convert --help` +soundcraft-cli info session.scraft +soundcraft-cli convert in.wav out.flac --bit-depth 16 --rate 44100 +soundcraft-cli run --demo --cmd 'mix.volume={"track":"Kick","db":-6}' --bounce mix.wav +soundcraft-cli script steps.txt --demo --keep-going --save out.scraft +soundcraft-cli commands bounce +soundcraft-cli describe file.bounce_mix +``` + +## Output and exit codes + +Results go to standard output, as JSON where a program is meant to read them. `info` (on an audio +file), `describe`, `app` and the `--inspect` output of `run` and `script` pretty-print it; `--compact` +prints the same JSON on one line. The result lines of `run` and `script` (`ID: JSON`) and the +`--bounce` and `--save` results are always one line each. `commands`, `plugins`, `parity` and the +`.scraft` summary of `info` are text. Errors go to standard error as `error: `. `mcp` writes only the protocol +to standard output. + +| Exit code | Meaning | +|---|---| +| 0 | Success | +| 1 | The command failed (a missing file, an unknown engine command, an app that is not running) | +| 2 | Usage error | + +Arguments are checked before any work starts. These are usage errors (exit 2): a flag the +subcommand does not take (`--fromat`), a flag given twice (only `--cmd` and the two automation +root flags repeat), a value of the wrong type or out of range, flags that conflict +(`--dither` with `--no-dither`, `--connect` with a root flag or `--demo`) and a missing value. A value flag +never takes the next flag as its value: write a value that starts with `-` as `--bounce=-x.wav`, or +give a file as `./-x.wav`. File names that are not valid Unicode reach the command unchanged. +The engine writes bounces and saved sessions through commands whose parameters are JSON text, so +those paths must be valid Unicode (`--bounce`, `--save`, and the `--in` session with +`--in-place`: exit 1). + +An output that is one of the inputs is a usage error too, however the path is spelt (`a.wav`, +`./a.wav`, a symbolic or hard link): + +| Subcommand | Inputs | Outputs | `--in-place` lets an output replace | +|---|---|---|---| +| `convert` | `IN` | `OUT` | `IN` | +| `run` | `--in` | `--bounce`, `--save` | the `--in` session, by `--save` only | +| `script` | `FILE`, `--in` | `--bounce`, `--save` | the `--in` session, by `--save` only; never `FILE` | + +`run` and `script` take `--in-place` only with `--in`. A `--bounce` naming the `--in` session, and +`--bounce` and `--save` naming one file, are refused with or without `--in-place`. With +`--in-place`, an output that is a symbolic link to the input replaces the file it links to; the +link stays. An existing file that is not an input is replaced without asking. + +## Environment variables + +| Variable | Flag | Notes | +|---|---|---| +| `SOUNDCRAFT_CONTROL_PORT` | `app --port` (CLI), `--control` (desktop app) | A port or `host:port`, an IPv6 host in brackets (`[::1]:7979`). The desktop app takes the port of a loopback `host:port` (`127.0.0.1`, `localhost`). Its control server listens on `127.0.0.1` only, so it ignores another host from the variable, with a log warning, and starts no control server; given to `--control`, another host is a usage error | + +- A flag wins over its variable. +- An empty variable counts as unset. +- A bad value is a usage error that names the variable. +- ` --help` lists the variable. +- The automation roots (`--automation-read-root`, `--automation-write-root`) are flags only. No + variable grants file access. + +`SOUNDCRAFT_CONFIG_DIR`, `SOUNDCRAFT_NO_PREFS`, `SOUNDCRAFT_NO_SYSTEM_FONTS`, `SOUNDCRAFT_WAYLAND` +(Linux) and `RUST_LOG` configure the desktop app (see the [README](../README.md#logs) and +[the control protocol](control-protocol.md)), not the CLI. `soundcraft --help` lists them. +An empty variable counts as unset (RFC-0012 rule 1.7), except `SOUNDCRAFT_CONFIG_DIR`: empty means +no settings folder. + +`SOUNDCRAFT_NO_PREFS`, `SOUNDCRAFT_NO_SYSTEM_FONTS` and `SOUNDCRAFT_WAYLAND` are on/off switches with +one rule: `1/on/true/yes` is on, `0/off/false/no` is off (any case, spaces ignored), and unset or +blank is the default (off). A value that isn't one of these spellings is ignored (the default applies) and logs a warning that +names the variable. + +## Desktop app launch flags + +`soundcraft [OPTIONS] [FILES]...` takes the same strict parsing and exit codes. +`soundcraft --help` lists the flags: + +| Flag | Meaning | +|---|---| +| `FILES` | Sessions, audio and MIDI files to open; put `--` before a name that starts with `-` | +| `--demo` (alias `--sample`) | Start with the demo session | +| `--no-audio` | Don't open an audio device | +| `--control ` | Start the control server on this port (`0` picks a free port and prints it); falls back to `SOUNDCRAFT_CONTROL_PORT` | +| `--version`, `-V` | Print the version | +| `--help`, `-h` | Print the flags and the environment variables | + +## Man pages and completions + +The Linux `.deb` and `.rpm` packages install `man soundcraft-cli` (plus a page per subcommand, such +as `man soundcraft-cli-convert`) and completions for bash, zsh and fish. The Linux and FreeBSD +tarballs carry the same files. The pages list each flag's environment variable, as `--help` does. +The desktop app has neither. + +Without a package, generate them from a source checkout: + +```sh +cargo run -p soundcraft-cli --example man-and-completions -- out/ +``` + +`out/` then holds `soundcraft-cli.bash`, `_soundcraft-cli` (zsh), `soundcraft-cli.fish`, +`soundcraft-cli.elv` (elvish), `_soundcraft-cli.ps1` (PowerShell) and the `.1` man pages. Copy +`soundcraft-cli.bash` to `~/.local/share/bash-completion/completions/soundcraft-cli`, +`soundcraft-cli.fish` into `~/.config/fish/completions/`, `_soundcraft-cli` into a directory on +zsh's `$fpath`, and read a page with `man -l out/soundcraft-cli.1`. + +## RFC-0012 + +soundcraft-cli follows [RFC-0012](https://github.com/storytold/craft-contrib/pull/18): clap derive +types, thiserror errors, `run(args, out, err)` behind `main`, environment variables declared on +their flags, generated completions and man pages, tests that pin help and every usage error, no +output replacing an input without `--in-place`, and the ast-grep rules in `lint/ast-grep`. The shared automation flags are declared once in +`crates/automation/src/args.rs`. + +Rules not followed yet: + +- **JSON flag (rule 1.3).** `commands`, `plugins`, `parity` and the `info` summary of a `.scraft` + session print text and have no `--json` flag. Whether such listings need one is an open question + in the RFC itself. +- **`string-error` rule (section 3).** Its ast-grep rule is on for the CLI code and the shared flags, + not yet for the rest of the workspace. diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 6b0c4932..470d3912 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -7,7 +7,7 @@ input), and every command is reachable from: | Surface | How | |---|---| | Desktop app control channel | `soundcraft --control ` (`0` = pick a free port; printed as `SOUNDCRAFT_CONTROL_PORT=…`) | -| CLI against a running app | `soundcraft-cli app --port [JSON]` | +| CLI against a running app | `soundcraft-cli app --port [JSON]` (see [cli.md](cli.md)) | | Headless CLI | `soundcraft-cli run --demo --cmd 'mix.volume={"track":"Kick","db":-6}' --bounce out.wav` | | MCP (headless) | `soundcraft-cli mcp [--demo]` | | MCP (bridged to the app) | `soundcraft-cli mcp --connect ` | diff --git a/docs/egui-037-migration-plan.md b/docs/egui-037-migration-plan.md index 4aa45323..d0b789de 100644 --- a/docs/egui-037-migration-plan.md +++ b/docs/egui-037-migration-plan.md @@ -1004,7 +1004,7 @@ Regression assertions: If unavailable, this is an explicit rollout blocker, not a pass. - FreeBSD: existing nightly workflow; compile/packaging can be checked, but native UI/IME/audio acceptance needs an actual FreeBSD environment (not provided here). -- Native control channel: use `--control 0` and `SOUNDCRAFT_NO_PREFS`/a scratch +- Native control channel: use `--control 0` and `SOUNDCRAFT_NO_PREFS=1`/a scratch `SOUNDCRAFT_CONFIG_DIR` for isolation; never arm/monitor/record in an automated UI run (`docs/control-protocol.md:22-25`). diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 924546d8..4d9009be 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -4,9 +4,9 @@ Every format Pro Tools reads or writes, and ours. The Pro Tools list comes from the document types its bundle declares (`CFBundleDocumentTypes` in Info.plist: names only, nothing else read) and its -documented import/bounce options. Ours comes from `crates/audio-io` (`detect_format`, `decode`, -`ENCODE_EXTENSIONS`), `crates/engine/src/cmd/file.rs`, `crates/engine/src/score.rs`, -`crates/engine/src/clip_group_file.rs` and `crates/video`. +documented import/bounce options. Ours comes from `crates/audio-io` (`FileFormat::READ` and +`FileFormat::WRITE`, `detect_format`, `decode`), `crates/engine/src/cmd/file.rs`, +`crates/engine/src/score.rs`, `crates/engine/src/clip_group_file.rs` and `crates/video`. **File formats: ~40 % ready** (estimated: weighted by how often a Pro Tools user hits each row; session interchange carries a third of the weight and is at 0 %). Remaining ≈ 70–120 h. diff --git a/docs/mcp.md b/docs/mcp.md index 93496258..26d367fd 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1,7 +1,8 @@ # MCP server SoundCraft speaks the [Model Context Protocol](https://modelcontextprotocol.io) over stdio, so -Claude and other agents can make music with it. +Claude and other agents can make music with it. The server is the `mcp` subcommand of +`soundcraft-cli`, which [docs/cli.md](cli.md) describes. ```sh soundcraft-cli mcp # headless engine (no window, no audio device) diff --git a/docs/releasing.md b/docs/releasing.md index 23d496de..b877757c 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -140,7 +140,12 @@ Locally on Windows: `dotnet tool install -g wix --version 5.0.2`, then `packaging/linux/package.sh` stages one FHS tree and makes every format from it: both binaries, `ai.storyteller.soundcraft.desktop` (with `MimeType=` for sessions, WAV, AIFF, FLAC, MP3 and MIDI), hicolor icons from 16 px to 512 px plus a scalable SVG, AppStream metainfo, and a -shared-mime-info file declaring `application/x-soundcraft-session` (`*.scraft`). +shared-mime-info file declaring `application/x-soundcraft-session` (`*.scraft`), and +`soundcraft-cli`'s man pages and bash, zsh and fish completions. The `man-and-completions` example, +built with the binaries, writes those; a cross build runs it through `EMULATOR` and leaves them out +without one. The zsh completion is in `/usr/share/zsh/site-functions`, which Fedora's and +openSUSE's zsh read; the `.deb` also links it from `/usr/share/zsh/vendor-completions`, which +Debian's zsh reads instead. - **AppImage** (maintained `AppImage/appimagetool`, static runtime: no libfuse2 needed). Each embeds the update information @@ -170,7 +175,8 @@ Locally (on Linux): install nfpm, then `packaging/linux/package.sh` (or `--forma GitHub has no FreeBSD runners, so the `freebsd` job builds inside a FreeBSD 14.3 VM (`vmactions/freebsd-vm`, pinned by commit) and runs `packaging/freebsd/package.sh`, which stages -a `/usr/local`-style tree (binaries, desktop entry, MIME type, metainfo, icons, docs) as a +a `/usr/local`-style tree (binaries, desktop entry, MIME type, metainfo, icons, docs, and +`soundcraft-cli`'s man pages and shell completions) as a `.tar.gz`. Install with `tar -xzf soundcraft-*-freebsd-x86_64.tar.gz --strip-components 1 -C /usr/local`. `.github/workflows/freebsd.yml` builds and tests the workspace there on every push to `main`. diff --git a/lint/ast-grep/rule-tests/args-os-test.yml b/lint/ast-grep/rule-tests/args-os-test.yml new file mode 100644 index 00000000..2e384b48 --- /dev/null +++ b/lint/ast-grep/rule-tests/args-os-test.yml @@ -0,0 +1,19 @@ +id: args-os +valid: + - let code = soundcraft_cli::run(std::env::args_os().skip(1), &mut out, &mut err); + - "let args: Vec = env::args_os().collect();" + - | + use std::env::args_os; + fn main() { let a: Vec<_> = args_os().collect(); } + - | + fn args() -> Vec { Vec::new() } + fn main() { let a = args(); } +invalid: + - "let args: Vec = std::env::args().collect();" + - for a in env::args().skip(1) {} + - | + use std::env::args; + fn main() { let a: Vec = args().collect(); } + - | + use std::env::{args, var}; + fn main() { for a in args() {} } diff --git a/lint/ast-grep/rule-tests/clap-error-message-test.yml b/lint/ast-grep/rule-tests/clap-error-message-test.yml new file mode 100644 index 00000000..385d46e9 --- /dev/null +++ b/lint/ast-grep/rule-tests/clap-error-message-test.yml @@ -0,0 +1,13 @@ +id: clap-error-message +valid: + - cmd.error(ErrorKind::ArgumentConflict, UsageError::BridgeRoots) + - cmd.error(ErrorKind::ArgumentConflict, Conflict { a, b }) + - sub.error(kind, self) + - log.error("not a clap error") +invalid: + - cmd.error(ErrorKind::ArgumentConflict, "configure automation roots on the desktop app") + - sub.error(ErrorKind::MissingRequiredArgument, format!("{flag} requires {transport}")) + - sub.error(ErrorKind::ArgumentConflict, BRIDGE_ROOTS) + - cmd.error(ErrorKind::InvalidValue, e.to_string()) + - cmd.error(ErrorKind::InvalidValue, String::from("bad value")) + - clap::Error::raw(ErrorKind::InvalidValue, "bad value") diff --git a/lint/ast-grep/rule-tests/clap-error-message-variable-test.yml b/lint/ast-grep/rule-tests/clap-error-message-variable-test.yml new file mode 100644 index 00000000..18d6e443 --- /dev/null +++ b/lint/ast-grep/rule-tests/clap-error-message-variable-test.yml @@ -0,0 +1,47 @@ +id: clap-error-message-variable +valid: + - | + fn conflict(cmd: &mut Command, a: String, b: String) -> clap::Error { + cmd.error(ErrorKind::ArgumentConflict, Conflict { a, b }) + } + - | + fn wrap(cmd: &mut Command, e: UsageError) -> clap::Error { + cmd.error(ErrorKind::ValueValidation, e) + } + - | + fn unrelated(cmd: &mut Command, e: UsageError) -> clap::Error { + let shown = format!("{e}"); + eprintln!("{shown}"); + cmd.error(ErrorKind::ValueValidation, e) + } + - | + fn cloned(cmd: &mut Command, e: &UsageError) -> clap::Error { + let owned = e.to_owned(); + cmd.error(ErrorKind::ValueValidation, owned) + } +invalid: + - | + fn conflict(cmd: &mut Command, a: &str, b: &str) -> clap::Error { + let message = format!("the argument '{a}' cannot be used with '{b}'"); + cmd.error(ErrorKind::ArgumentConflict, message) + } + - | + fn bad(cmd: &mut Command, e: UsageError) -> clap::Error { + let message = e.to_string(); + cmd.error(ErrorKind::ValueValidation, message) + } + - | + fn typed(cmd: &mut Command, flag: &str) -> clap::Error { + let message: String = flag.into(); + clap::Error::raw(ErrorKind::InvalidValue, message) + } + - | + fn literal(cmd: &mut Command) -> clap::Error { + let message = "bad value".to_owned(); + cmd.error(ErrorKind::InvalidValue, message) + } + - | + fn from(cmd: &mut Command) -> clap::Error { + let message = String::from("bad value"); + cmd.error(ErrorKind::InvalidValue, message) + } diff --git a/lint/ast-grep/rule-tests/string-error-test.yml b/lint/ast-grep/rule-tests/string-error-test.yml new file mode 100644 index 00000000..4f14197c --- /dev/null +++ b/lint/ast-grep/rule-tests/string-error-test.yml @@ -0,0 +1,14 @@ +id: string-error +valid: + - "fn parse(s: &str) -> Result { todo!() }" + - "fn name() -> Option { None }" + - "fn ok() -> Result { Ok(String::new()) }" + - Err(PortError::Range) + - Err(e) +invalid: + - "fn parse(s: &str) -> Result { todo!() }" + - "type R = Result<(), String>;" + - Err(format!("bad port {s}")) + - Err(e.to_string()) + - Err(String::from("bad port")) + - Err("bad port") diff --git a/lint/ast-grep/rules/args-os.yml b/lint/ast-grep/rules/args-os.yml new file mode 100644 index 00000000..a852cfcc --- /dev/null +++ b/lint/ast-grep/rules/args-os.yml @@ -0,0 +1,21 @@ +id: args-os +language: rust +severity: error +message: std::env::args() panics on an argument that isn't valid Unicode +note: | + Read std::env::args_os() and let clap parse the OsStrings, so a Latin-1 file name reaches the + command (or is a usage error) instead of aborting before anything is printed. +rule: + any: + - pattern: std::env::args() + - pattern: env::args() + # A bare args() only where the file imports std::env::args, not any function named args. + - all: + - pattern: args() + - inside: + kind: source_file + stopBy: end + has: + kind: use_declaration + stopBy: end + regex: std::env::(args\b|\{[^}]*\bargs\b) diff --git a/lint/ast-grep/rules/clap-error-message-variable.yml b/lint/ast-grep/rules/clap-error-message-variable.yml new file mode 100644 index 00000000..3f476be4 --- /dev/null +++ b/lint/ast-grep/rules/clap-error-message-variable.yml @@ -0,0 +1,35 @@ +id: clap-error-message-variable +language: rust +severity: error +message: clap error built from a string held in a variable +note: | + The message was built with format!, .to_string(), String::from or a literal's .to_owned() a + few lines up. Give it a variant on a + #[derive(thiserror::Error)] enum instead, with the parts as fields, and pass the variant. +rule: + # In this order: the call binds $MSG, then the function must hold a `let` of that same name. + all: + - any: + - pattern: $CMD.error($KIND, $MSG) + - pattern: Error::raw($KIND, $MSG) + - pattern: clap::Error::raw($KIND, $MSG) + - inside: + kind: function_item + stopBy: end + has: + stopBy: end + any: + - pattern: let $MSG = format!($$$); + - pattern: "let $MSG: String = $VAL;" + - pattern: let $MSG = $SRC.to_string(); + - pattern: let $MSG = String::from($$$); + # .to_owned() only on a string literal: on anything else it may clone a typed error. + - all: + - kind: let_declaration + - regex: '=\s*"(?s:.*)"\s*\.to_owned\(\)\s*;?$' + - has: + field: pattern + pattern: $MSG +constraints: + MSG: + kind: identifier diff --git a/lint/ast-grep/rules/clap-error-message.yml b/lint/ast-grep/rules/clap-error-message.yml new file mode 100644 index 00000000..e18b824b --- /dev/null +++ b/lint/ast-grep/rules/clap-error-message.yml @@ -0,0 +1,26 @@ +id: clap-error-message +language: rust +severity: error +message: clap error built from a string +note: | + Give the message a variant on a #[derive(thiserror::Error)] enum and pass the variant, e.g. + `cmd.error(ErrorKind::ArgumentConflict, UsageError::BridgeRoots)`. A string literal, format!, + .to_string() or a SCREAMING_CASE const keeps the message apart from the error's type. +rule: + any: + - pattern: $CMD.error($KIND, $MSG) + - pattern: Error::raw($KIND, $MSG) + - pattern: clap::Error::raw($KIND, $MSG) +constraints: + MSG: + any: + - kind: string_literal + - all: + - kind: macro_invocation + - regex: ^format! + - all: + - kind: identifier + - regex: ^[A-Z][A-Z0-9_]*$ + - all: + - kind: call_expression + - regex: \.(to_string|to_owned)\(\)$|^String::from\( diff --git a/lint/ast-grep/rules/string-error.yml b/lint/ast-grep/rules/string-error.yml new file mode 100644 index 00000000..e79b83d0 --- /dev/null +++ b/lint/ast-grep/rules/string-error.yml @@ -0,0 +1,21 @@ +id: string-error +language: rust +severity: error +message: String as an error type +note: | + Return a #[derive(thiserror::Error)] enum, so a caller can match on what failed and the + messages live with their variants. +rule: + any: + - pattern: + context: type T = Result<$OK, String>; + selector: generic_type + - pattern: Err(format!($$$)) + - pattern: Err($S.to_string()) + - pattern: Err(String::from($$$)) + - all: + - pattern: Err($S) + - has: + kind: arguments + has: + kind: string_literal diff --git a/lint/ast-grep/sgconfig.yml b/lint/ast-grep/sgconfig.yml new file mode 100644 index 00000000..9c94bd0a --- /dev/null +++ b/lint/ast-grep/sgconfig.yml @@ -0,0 +1,6 @@ +# Structural rules for the command-line code (RFC-0012), run by .github/workflows/ast-grep.yml. +# Each rule in rules/ has its cases in rule-tests/, which CI checks before the scan. +ruleDirs: + - rules +testConfigs: + - testDir: rule-tests diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index 9ec31a55..98d86199 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -2,8 +2,8 @@ # Build and package SoundCraft for FreeBSD: # # $DIST/soundcraft--freebsd-x86_64.tar.gz a /usr/local-style tree: -# soundcraft--freebsd-x86_64/{bin, share/applications, share/icons, share/mime, -# share/metainfo, share/doc/soundcraft} +# soundcraft--freebsd-x86_64/{bin, man/man1, share/applications, share/icons, +# share/mime, share/metainfo, share/doc/soundcraft, share/{bash-completion,zsh,fish}} # # Install by copying the tree's contents into /usr/local, e.g. # tar -xzf soundcraft-*-freebsd-x86_64.tar.gz --strip-components 1 -C /usr/local @@ -55,8 +55,16 @@ BASENAME="soundcraft-$VERSION-freebsd-$ARCH" echo "==> SoundCraft $VERSION for FreeBSD $ARCH" +# The man-and-completions example is built with the binaries: it writes soundcraft-cli's man pages +# and shell completions below. A cross build's example cannot run here, so it is built again for +# this machine (the pages and completions do not depend on the target). if [[ "$SKIP_BUILD" = 0 ]]; then - (cd "$ROOT" && cargo build --release --locked -p soundcraft -p soundcraft-cli) + (cd "$ROOT" && cargo build --release --locked -p soundcraft -p soundcraft-cli \ + --bins --example man-and-completions) + if [[ -n "${CARGO_BUILD_TARGET:-}" ]]; then + (cd "$ROOT" && env -u CARGO_BUILD_TARGET cargo build --release --locked -p soundcraft-cli \ + --example man-and-completions) + fi fi # Cargo puts a cross build under target//release. BIN="$CARGO_TARGET_DIR/${CARGO_BUILD_TARGET:+$CARGO_BUILD_TARGET/}release" @@ -76,6 +84,20 @@ sed -e "s/@VERSION@/$VERSION/g" -e "s/@DATE@/$SOUNDCRAFT_BUILD_DATE/g" \ "$LINUX/$APP_ID.metainfo.xml.in" >"$STAGE/share/metainfo/$APP_ID.metainfo.xml" cp -R "$ROOT/assets/app-icon/hicolor" "$STAGE/share/icons/" copy_docs "$STAGE/share/doc/soundcraft" +# soundcraft-cli's man pages and shell completions, from its clap definition, written by the +# example built above. +CLI_DOCS="$CARGO_TARGET_DIR/release/examples/man-and-completions" +if [[ ! -x "$CLI_DOCS" ]]; then + echo "error: $CLI_DOCS is missing: build it with the binaries (without --skip-build)" >&2 + exit 1 +fi +"$CLI_DOCS" "$WORK/cli-docs" +mkdir -p "$STAGE/man/man1" "$STAGE/share/bash-completion/completions" \ + "$STAGE/share/zsh/site-functions" "$STAGE/share/fish/vendor_completions.d" +install -m 644 "$WORK"/cli-docs/*.1 "$STAGE/man/man1/" +install -m 644 "$WORK/cli-docs/soundcraft-cli.bash" "$STAGE/share/bash-completion/completions/soundcraft-cli" +install -m 644 "$WORK/cli-docs/_soundcraft-cli" "$STAGE/share/zsh/site-functions/_soundcraft-cli" +install -m 644 "$WORK/cli-docs/soundcraft-cli.fish" "$STAGE/share/fish/vendor_completions.d/soundcraft-cli.fish" # ---- .tar.gz ------------------------------------------------------------------------------------- tar -C "$WORK" -czf "$DIST/$BASENAME.tar.gz" "$BASENAME" diff --git a/packaging/linux/nfpm.yaml b/packaging/linux/nfpm.yaml index c9f2eb78..55f040e6 100644 --- a/packaging/linux/nfpm.yaml +++ b/packaging/linux/nfpm.yaml @@ -69,6 +69,12 @@ contents: - src: ${STAGE}/usr/share/ dst: /usr/share/ type: tree + # Debian's zsh reads vendor-completions, not /usr/share/zsh/site-functions as Fedora's and + # openSUSE's do (its debian/rules sets --enable-additional-fpath). + - src: /usr/share/zsh/site-functions/_soundcraft-cli + dst: /usr/share/zsh/vendor-completions/_soundcraft-cli + type: symlink + packager: deb scripts: postinstall: packaging/linux/postinst.sh diff --git a/packaging/linux/package.sh b/packaging/linux/package.sh index f27357be..aa4d577a 100755 --- a/packaging/linux/package.sh +++ b/packaging/linux/package.sh @@ -16,7 +16,8 @@ # # Cross-compiling (CI's linux-riscv64 job, tar.gz only): set CROSS_ARCH (riscv64), CROSS_TARGET # (the Rust target), CROSS_COMPILE (the binutils prefix, for strip) and optionally EMULATOR -# (e.g. qemu-riscv64) to smoke-test the CLI. +# (e.g. qemu-riscv64) to smoke-test the CLI and run the example that writes its man pages and +# completions. set -euo pipefail # shellcheck source=../env.sh . "$(dirname "${BASH_SOURCE[0]}")/../env.sh" @@ -29,7 +30,7 @@ while [[ $# -gt 0 ]]; do case "$1" in --skip-build) SKIP_BUILD=1; shift ;; --formats) FORMATS="$2"; shift 2 ;; - -h | --help) sed -n '2,19p' "$0"; exit 0 ;; + -h | --help) sed -n '2,20p' "$0"; exit 0 ;; *) echo "unknown argument: $1" >&2; exit 2 ;; esac done @@ -46,11 +47,15 @@ BASENAME="soundcraft-$VERSION-linux-$ARCH" echo "==> SoundCraft $VERSION for Linux $ARCH ($FORMATS)" +# The man-and-completions example is built with the binaries: it writes soundcraft-cli's man pages +# and shell completions below. if [[ "$SKIP_BUILD" = 0 ]]; then if [[ -n "${CROSS_TARGET:-}" ]]; then - (cd "$ROOT" && cargo build --release --locked -p soundcraft -p soundcraft-cli --target "$CROSS_TARGET") + (cd "$ROOT" && cargo build --release --locked -p soundcraft -p soundcraft-cli \ + --bins --example man-and-completions --target "$CROSS_TARGET") else - (cd "$ROOT" && cargo build --release --locked -p soundcraft -p soundcraft-cli) + (cd "$ROOT" && cargo build --release --locked -p soundcraft -p soundcraft-cli \ + --bins --example man-and-completions) fi fi BIN="$CARGO_TARGET_DIR/${CROSS_TARGET:+$CROSS_TARGET/}release" @@ -71,6 +76,28 @@ mkdir -p "$STAGE/usr/share/icons" cp -R "$ROOT/assets/app-icon/hicolor" "$STAGE/usr/share/icons/" mkdir -p "$STAGE/usr/share/doc/soundcraft" copy_docs "$STAGE/usr/share/doc/soundcraft" +# soundcraft-cli's man pages and shell completions, from its clap definition, written by the +# example built above. A cross build runs it through $EMULATOR, like the --version check at the +# end, and leaves them out without one. zsh reads site-functions on Fedora and openSUSE; the deb +# also links the completion into Debian's vendor-completions (nfpm.yaml). +CLI_DOCS="$BIN/examples/man-and-completions" +if [[ -n "${CROSS_TARGET:-}" ]] && [[ -z "${EMULATOR:-}" ]]; then + warn "no EMULATOR to run the $CROSS_TARGET build, so soundcraft-cli's man pages and completions are left out" +else + if [[ ! -x "$CLI_DOCS" ]]; then + echo "error: $CLI_DOCS is missing: build it with the binaries (without --skip-build)" >&2 + exit 1 + fi + if [[ -n "${CROSS_TARGET:-}" ]]; then + "$EMULATOR" "$CLI_DOCS" "$WORK/cli-docs" + else + "$CLI_DOCS" "$WORK/cli-docs" + fi + install -Dm644 -t "$STAGE/usr/share/man/man1" "$WORK"/cli-docs/*.1 + install -Dm644 "$WORK/cli-docs/soundcraft-cli.bash" "$STAGE/usr/share/bash-completion/completions/soundcraft-cli" + install -Dm644 "$WORK/cli-docs/_soundcraft-cli" "$STAGE/usr/share/zsh/site-functions/_soundcraft-cli" + install -Dm644 "$WORK/cli-docs/soundcraft-cli.fish" "$STAGE/usr/share/fish/vendor_completions.d/soundcraft-cli.fish" +fi if command -v desktop-file-validate >/dev/null; then desktop-file-validate "$STAGE/usr/share/applications/$APP_ID.desktop" @@ -98,7 +125,11 @@ if has deb || has rpm; then export VERSION export NFPM_ARCH="$DEB_ARCH" # nfpm expands env vars in fields like `version` and `arch`, but not in `contents[].src`. - sed "s|\${STAGE}|$STAGE|g" "$HERE/nfpm.yaml" >"$WORK/nfpm.yaml" + # Without the completions (a cross build with no EMULATOR) the deb's zsh symlink would dangle, + # so its entry is dropped too. + DROP_ZSH_LINK= + [[ -f "$STAGE/usr/share/zsh/site-functions/_soundcraft-cli" ]] || DROP_ZSH_LINK='\|^ - src: /usr/share/zsh/site-functions/|,/packager: deb/d;' + sed "${DROP_ZSH_LINK}s|\${STAGE}|$STAGE|g" "$HERE/nfpm.yaml" >"$WORK/nfpm.yaml" for fmt in deb rpm; do if has "$fmt"; then (cd "$ROOT" && nfpm package -f "$WORK/nfpm.yaml" -p "$fmt" -t "$DIST/$BASENAME.$fmt"); fi done