Clean-up

src/enrich.rs

@@ -815,17 +815,8 @@ fn urlencoded(s: &str) -> String {
 // Main enrichment function
 // ---------------------------------------------------------------------------
 
-/// Run the enrichment process: find all uncached albums and fetch their
-/// metadata and cover art from MusicBrainz / Cover Art Archive.
-///
-/// Progress is printed to stderr so the user can see what's happening.
-///
-/// # Arguments
-/// - `conn` — SQLite database connection
-/// - `force` — if true, re-fetch all albums even if already cached
-/// - `quiet` — if true, suppress the "nothing to do" hint (used when called
-///   from `report --html` where the user didn't explicitly ask for enrichment)
 /// Enrich only the albums in `needed` that aren't already cached.
+///
 /// Used by `report --html` so we fetch covers only for what the report
 /// will actually display, rather than the entire scrobble library.
 pub fn run_enrich_targeted(
@@ -848,6 +839,17 @@ pub fn run_enrich_targeted(
     run_enrich_albums(conn, albums, quiet);
 }
 
+/// Run the enrichment process: find all uncached albums and fetch their
+/// metadata and cover art from MusicBrainz / Cover Art Archive.
+///
+/// Progress is printed to stderr so the user can see what's happening.
+///
+/// # Arguments
+///
+/// - `conn` — SQLite database connection
+/// - `force` — if true, re-fetch all albums even if already cached
+/// - `quiet` — if true, suppress the "nothing to do" hint (used when called
+///   from `report --html` where the user didn't explicitly ask for enrichment)
 pub fn run_enrich(conn: &Connection, force: bool, quiet: bool) {
     if force {
         conn.execute("DELETE FROM album_cache", [])

src/main.rs

@@ -32,9 +32,9 @@
 //!   [playerctl status]   ──reader thread──→ ┘
 //! ```
 //!
-//! ### MPD watcher (optional, enabled with `--mpd`)
+//! ### MPD watcher (on by default, disabled with `--no-mpd`)
 //!
-//! When `--mpd` is passed, a separate thread is spawned that connects to MPD
+//! Unless `--no-mpd` is passed, a separate thread is spawned that connects to MPD
 //! using the idle protocol. It maintains its own `ScrobbleTracker` and writes
 //! to the same database. The two watchers are fully independent — neither
 //! knows about the other, and no synchronisation is needed between them.
@@ -404,6 +404,7 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
     // When MPRIS is disabled, this loop exits immediately on shutdown.
     let mut tracker = watcher::create_db_tracker(conn);
     let mut eof_count = 0;
+    let mut flushed = false;
 
     while running.load(Ordering::SeqCst) {
         match rx.recv() {
@@ -415,6 +416,7 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
                     // sends one Eof, so we may receive more than `expected_eofs`.
                     if eof_count >= expected_eofs {
                         tracker.handle_event(watcher::Event::Eof);
+                        flushed = true;
                         break;
                     }
                     continue;
@@ -426,9 +428,11 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
         }
     }
 
-    // Final flush: safe even if Eof was already handled — the tracker
+    // Final flush in case the loop exited via channel disconnect rather than
-    // is a no-op when there is no current track.
+    // the normal Eof path above (e.g., all senders dropped simultaneously).
+    if !flushed {
         tracker.handle_event(watcher::Event::Eof);
+    }
 
     // Wait for the MPD watcher thread to finish its graceful shutdown.
     // It will notice `running == false` on the next idle timeout (≤ 500 ms).

src/mpd.rs

@@ -100,13 +100,13 @@ use crate::watcher::{self, Event, PlayerStatus};
 ///
 /// ```
 /// // Standard localhost installation
-/// let cfg = MpdConfig::default_tcp();
+/// let cfg = MpdConfig { host: "localhost".into(), port: 6600 };
 ///
 /// // Custom port
 /// let cfg = MpdConfig { host: "localhost".into(), port: 6601 };
 ///
 /// // Unix socket (when MPD runs under a different user account)
-/// let cfg = MpdConfig { host: "/run/user/1000/mpd/socket".into(), port: 0 };
+/// let cfg = MpdConfig { host: "/run/mpd/socket".into(), port: 0 };
 /// ```
 #[derive(Debug, Clone)]
 pub struct MpdConfig {
@@ -578,17 +578,17 @@ impl MpdPlayerState {
     }
 }
 
-/// Run the MPD watcher loop. This function blocks until `running` is set to
+/// Run the MPD watcher reconnect-and-event loop.
-/// `false` (e.g. by a Ctrl+C handler) or MPD becomes persistently unreachable.
+///
+/// Connects to MPD, then loops waiting for player events. If the connection
+/// drops (MPD restarted, network hiccup), waits 5 seconds and reconnects
+/// automatically. Blocks until `running` is set to `false` (e.g. by a
+/// Ctrl+C handler), at which point it flushes any in-progress scrobble
+/// and returns.
 ///
 /// It maintains its own [`watcher::ScrobbleTracker`] backed by the supplied
 /// database connection. Scrobbles are written to the same SQLite database as
 /// the MPRIS watcher, so the two sources appear together in reports.
-///
-/// ## Reconnection
-///
-/// If the MPD connection drops (MPD restarted, network hiccup), this function
-/// waits 5 seconds and retries. It gives up after the shutdown flag is set.
 pub fn run_mpd_watch(config: &MpdConfig, conn: Arc<Mutex<Connection>>, running: Arc<AtomicBool>) {
     eprintln!(
         "[mpd] Connecting to MPD at {}{}",
@@ -692,7 +692,7 @@ fn run_event_loop(
         // either get the player event or the shutdown flag is set.
         let changed = wait_for_idle_response(mpd_conn, running);
         match changed {
-            IdleResult::PlayerChanged => {}
+            IdleResult::Wakeup => {}
             IdleResult::Shutdown => return true,
             IdleResult::ConnectionLost => return false,
         }
@@ -711,7 +711,11 @@ fn run_event_loop(
 
 /// The three possible outcomes of waiting for an MPD idle response.
 enum IdleResult {
-    PlayerChanged,
+    /// The idle wait ended and state should be re-queried. This covers both
+    /// the normal case (`changed: player` received) and the degenerate case
+    /// where MPD returns `OK` without a `changed` line (e.g., a different
+    /// subsystem fired despite `idle player` — unusual but handled safely).
+    Wakeup,
     Shutdown,
     ConnectionLost,
 }
@@ -722,27 +726,26 @@ enum IdleResult {
 /// Because the socket has a 500 ms read timeout, `read_line` returns a
 /// `TimedOut`/`WouldBlock` error periodically. We use those timeouts to check
 /// the shutdown flag without blocking for the full duration of a track.
+///
+/// Returns [`IdleResult::Wakeup`] when the response terminates — regardless
+/// of whether a `changed: player` line appeared. In both cases the caller
+/// re-queries MPD state, so the distinction doesn't matter.
 fn wait_for_idle_response(conn: &mut MpdConn, running: &Arc<AtomicBool>) -> IdleResult {
-    let mut saw_changed_player = false;
-
     loop {
         let mut line = String::new();
         match conn.reader.read_line(&mut line) {
             Ok(0) => return IdleResult::ConnectionLost, // EOF
             Ok(_) => {
                 let trimmed = line.trim();
-                if trimmed == "changed: player" {
+                if trimmed == "OK" {
-                    saw_changed_player = true;
+                    // End of idle response — re-query state regardless of whether
-                } else if trimmed == "OK" {
+                    // "changed: player" appeared. With `idle player` the line
-                    // End of idle response — if we saw "changed: player", report it.
+                    // should always appear, but if it's absent (degenerate case)
-                    if saw_changed_player {
+                    // a spurious re-query is harmless.
-                        return IdleResult::PlayerChanged;
+                    return IdleResult::Wakeup;
-                    }
-                    // "OK" with no "changed: player" means the idle was cancelled
-                    // without a relevant event (e.g., a different subsystem changed).
-                    // Re-enter idle by returning and letting the outer loop retry.
-                    return IdleResult::PlayerChanged; // Re-query state regardless.
                 }
+                // "changed: player" and any other key/value lines are consumed
+                // and discarded here — we query currentsong/status ourselves.
             }
             Err(ref e)
                 if e.kind() == io::ErrorKind::WouldBlock || e.kind() == io::ErrorKind::TimedOut =>

src/report.rs

@@ -453,15 +453,6 @@ fn truncate_cell(s: &str, width: usize) -> String {
 // Report generation
 // ---------------------------------------------------------------------------
 
-/// Query the database and assemble all sections of the report.
-///
-/// This is the main entry point for report generation. It runs all the
-/// necessary SQL queries for the given period and limit, and returns
-/// a `ReportData` struct ready for rendering.
-///
-/// # Arguments
-/// - `conn`   — SQLite database connection
-/// - `period` — time period filter ("today", "week", "month", "year", "all")
 /// Return the set of `(artist, album)` pairs that will appear anywhere in the
 /// HTML report at the given limits. Used to focus automatic enrichment on
 /// exactly what the report needs rather than the entire scrobble library.
@@ -506,6 +497,16 @@ pub fn albums_needed_for_report(
     pairs
 }
 
+/// Query the database and assemble all sections of the report.
+///
+/// This is the main entry point for report generation. It runs all the
+/// necessary SQL queries for the given period and limit, and returns
+/// a `ReportData` struct ready for rendering.
+///
+/// # Arguments
+///
+/// - `conn`   — SQLite database connection
+/// - `period` — time period filter ("today", "week", "month", "year", "all")
 /// - `limit`  — maximum number of entries in each top-N list
 pub fn gather_report(
     conn: &Connection,