From bf4b7550e78fc9d61c09e93f96d82cae81fc3469 Mon Sep 17 00:00:00 2001 From: Pushkar Patel Date: Tue, 10 Mar 2026 07:44:52 +0530 Subject: [PATCH 1/3] fix: handle missing playback dependency --- README.md | 25 ++++++++++++++++++-- src/deps.rs | 4 ++++ src/main.rs | 17 +++++++++----- src/playback.rs | 19 ++++++++++++++++ src/tui/mod.rs | 58 +++++++++++++++++++++++++++++++++++++---------- src/tui/triage.rs | 15 ++++++++---- tests/cli.rs | 14 ++++++++++++ 7 files changed, 127 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 55b63ee..fa92b7e 100644 --- a/README.md +++ b/README.md @@ -21,12 +21,30 @@ mls ~/Videos | jq . # streaming NDJSON brew install thepushkarp/tap/mls ``` +The Homebrew formula installs `ffmpeg` and `mpv`, so probing and playback work +after a fresh Brew install. Install `trash` separately if you want safe delete +in triage mode: + +```bash +brew install trash +``` + ### Cargo ```bash cargo install media-ls ``` +`cargo install` only installs the `mls` binary. Install runtime tools +separately for probe/playback support: + +```bash +brew install ffmpeg mpv + +# Optional: safe delete in triage mode +brew install trash +``` + ### Build from source ```bash @@ -36,10 +54,13 @@ cargo build --release # requires Rust 1.85+ cp target/release/mls ~/.local/bin/ # or anywhere on PATH ``` -### Prerequisites +For source builds, install the same runtime tools as the Cargo route: ```bash -brew install ffmpeg mpv trash +brew install ffmpeg mpv + +# Optional: safe delete in triage mode +brew install trash ``` | Dependency | Required | Purpose | diff --git a/src/deps.rs b/src/deps.rs index ffe5add..edb1d47 100644 --- a/src/deps.rs +++ b/src/deps.rs @@ -6,6 +6,10 @@ use std::fmt::Write; use std::process::Command as StdCommand; +pub const PLAYBACK_REQUIRES_MPV: &str = "Playback requires mpv. Install: brew install mpv"; +pub const PLAYBACK_DISABLED_WARNING: &str = + "Warning: mpv not found. Playback features disabled. Install: brew install mpv"; + /// Result of checking external dependencies. #[derive(Debug)] pub struct DepCheck { diff --git a/src/main.rs b/src/main.rs index 2f59808..3dd2696 100644 --- a/src/main.rs +++ b/src/main.rs @@ -78,12 +78,17 @@ async fn run() -> Result<()> { .into()); } - // Warn about optional deps (mpv) - if dep_check.mpv.is_none() && !cli.quiet { - let _ = writeln!( - std::io::stderr(), - "Warning: mpv not found. Playback features disabled. Install: brew install mpv" - ); + if dep_check.mpv.is_none() { + if matches!(&cli.command, Some(Command::Play { .. })) { + return Err(ExitCodeError { + code: exit_code::DEPENDENCY, + msg: deps::PLAYBACK_REQUIRES_MPV.into(), + } + .into()); + } + if !cli.quiet { + let _ = writeln!(std::io::stderr(), "{}", deps::PLAYBACK_DISABLED_WARNING); + } } // Route to subcommand diff --git a/src/playback.rs b/src/playback.rs index 59a9f3d..0d13376 100644 --- a/src/playback.rs +++ b/src/playback.rs @@ -32,6 +32,25 @@ pub struct MpvController { conn: Option, } +/// Convert playback startup failures into a user-facing status string. +#[must_use] +pub fn playback_error_message(error: &anyhow::Error) -> String { + if error + .to_string() + .contains(crate::deps::PLAYBACK_REQUIRES_MPV) + { + return crate::deps::PLAYBACK_REQUIRES_MPV.to_string(); + } + if error + .chain() + .filter_map(|cause| cause.downcast_ref::()) + .any(|io_error| io_error.kind() == std::io::ErrorKind::NotFound) + { + return crate::deps::PLAYBACK_REQUIRES_MPV.to_string(); + } + format!("Playback failed: {error}") +} + impl MpvController { /// Create a new controller (mpv not yet spawned). #[must_use] diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 7f88a9d..86f5c34 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -403,6 +403,27 @@ impl App { self.status_ticks = 30; } + fn clear_playback_state(&mut self) { + self.playback_file_name = None; + self.playback_position = None; + self.playback_duration = None; + } + + fn apply_playback_start_result(&mut self, result: Result<()>, name: &str) { + match result { + Ok(()) => { + self.playback_file_name = Some(name.to_string()); + self.playback_position = None; + self.playback_duration = None; + self.set_status(format!("Playing: {name}")); + } + Err(error) => { + self.clear_playback_state(); + self.set_status(crate::playback::playback_error_message(&error)); + } + } + } + /// Remove the currently selected media entry from the entries list. /// /// Called after successful delete or move in triage mode so the @@ -726,9 +747,7 @@ async fn event_loop( // Update mpv state — detect process exit if app.mpv.state() != PlaybackState::Stopped && !app.mpv.is_alive() { app.mpv.stop().await; - app.playback_position = None; - app.playback_duration = None; - app.playback_file_name = None; + app.clear_playback_state(); } // Poll playback position only while actively playing (not paused/stopped) @@ -760,7 +779,6 @@ async fn event_loop( Ok(()) } -#[expect(clippy::too_many_lines, reason = "match arms for key handling")] async fn handle_key(app: &mut App, key: KeyEvent) { // Handle filter input mode if app.filter_active { @@ -840,9 +858,7 @@ async fn handle_key(app: &mut App, key: KeyEvent) { (KeyCode::Char('p'), _) => handle_playback(app).await, (KeyCode::Char('P'), _) => { app.mpv.stop().await; - app.playback_position = None; - app.playback_duration = None; - app.playback_file_name = None; + app.clear_playback_state(); app.set_status("Stopped playback".to_string()); } (KeyCode::Char(']'), _) => { @@ -946,11 +962,8 @@ async fn handle_playback(app: &mut App) { let _ = app.mpv.toggle_pause().await; } else { // Stopped or different file — start playing selected - let _ = app.mpv.play(&path, audio_only).await; - app.playback_file_name = Some(name.clone()); - app.playback_position = None; - app.playback_duration = None; - app.set_status(format!("Playing: {name}")); + let result = app.mpv.play(&path, audio_only).await; + app.apply_playback_start_result(result, &name); } } @@ -1448,6 +1461,27 @@ mod tests { assert!(app.playback_file_name.is_none()); } + #[test] + fn playback_start_failure_keeps_state_empty_and_sets_error_status() { + let mut app = make_test_app(&["a.mp4"]); + let name = "a.mp4"; + + app.apply_playback_start_result( + Err(anyhow::anyhow!( + "Playback requires mpv. Install: brew install mpv" + )), + name, + ); + + assert!(app.playback_file_name.is_none()); + assert!(app.playback_position.is_none()); + assert!(app.playback_duration.is_none()); + assert_eq!( + app.status_message.as_deref(), + Some("Playback requires mpv. Install: brew install mpv") + ); + } + #[test] fn thumb_skips_audio_only_files() { // make_entry creates entries with video: None (audio-only) diff --git a/src/tui/triage.rs b/src/tui/triage.rs index ef44f24..e0a066b 100644 --- a/src/tui/triage.rs +++ b/src/tui/triage.rs @@ -207,11 +207,16 @@ pub async fn handle_triage_key(app: &mut App, key: KeyEvent) { // Playback in triage KeyCode::Char('p') => { if app.mpv.state() == crate::playback::PlaybackState::Stopped { - let info = app - .selected_entry() - .map(|entry| (entry.path.clone(), entry.media.video.is_none())); - if let Some((path, audio_only)) = info { - let _ = app.mpv.play(&path, audio_only).await; + let info = app.selected_entry().map(|entry| { + ( + entry.path.clone(), + entry.media.video.is_none(), + entry.file_name.clone(), + ) + }); + if let Some((path, audio_only, name)) = info { + let result = app.mpv.play(&path, audio_only).await; + app.apply_playback_start_result(result, &name); } } else { let _ = app.mpv.toggle_pause().await; diff --git a/tests/cli.rs b/tests/cli.rs index d8ace95..5a47cf9 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -81,6 +81,20 @@ fn missing_ffprobe_exits_4() { .code(4); } +#[test] +fn play_without_mpv_exits_4_with_install_hint() { + let tmp = setup_media_dir(); + Command::new(cargo_bin("mls")) + .env("PATH", mock_bin_dir()) + .arg("--quiet") + .arg("play") + .arg(tmp.path().join("song.mp3")) + .assert() + .code(4) + .stderr(predicate::str::contains("Playback requires mpv")) + .stderr(predicate::str::contains("brew install mpv")); +} + // --- Validation errors --- #[test] From fa728d98bc01b12a64daea3926e4665a210ee075 Mon Sep 17 00:00:00 2001 From: Pushkar Patel Date: Tue, 10 Mar 2026 07:47:09 +0530 Subject: [PATCH 2/3] chore: bump version to 0.0.3 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8523749..8203f3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1206,7 +1206,7 @@ dependencies = [ [[package]] name = "media-ls" -version = "0.0.2" +version = "0.0.3" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index 0fdc284..001ec63 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "media-ls" -version = "0.0.2" +version = "0.0.3" edition = "2024" description = "Media LS — terminal-native audio/video file browser with metadata columns, TUI preview, and structured JSON output" license = "MIT" From a6d07242b6e2c5e21175fac33d0e2fd60c5226a0 Mon Sep 17 00:00:00 2001 From: Pushkar Patel Date: Tue, 10 Mar 2026 07:58:42 +0530 Subject: [PATCH 3/3] refactor: simplify playback error detection --- src/playback.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/playback.rs b/src/playback.rs index 0d13376..5e8b37e 100644 --- a/src/playback.rs +++ b/src/playback.rs @@ -35,20 +35,19 @@ pub struct MpvController { /// Convert playback startup failures into a user-facing status string. #[must_use] pub fn playback_error_message(error: &anyhow::Error) -> String { - if error + let is_mpv_missing = error .to_string() .contains(crate::deps::PLAYBACK_REQUIRES_MPV) - { - return crate::deps::PLAYBACK_REQUIRES_MPV.to_string(); + || error + .chain() + .filter_map(|cause| cause.downcast_ref::()) + .any(|io_error| io_error.kind() == std::io::ErrorKind::NotFound); + + if is_mpv_missing { + crate::deps::PLAYBACK_REQUIRES_MPV.to_string() + } else { + format!("Playback failed: {error}") } - if error - .chain() - .filter_map(|cause| cause.downcast_ref::()) - .any(|io_error| io_error.kind() == std::io::ErrorKind::NotFound) - { - return crate::deps::PLAYBACK_REQUIRES_MPV.to_string(); - } - format!("Playback failed: {error}") } impl MpvController {