From 400d3b1e3af0ce2790eae9ee54a0668ea3777bba Mon Sep 17 00:00:00 2001 From: mierak <47547062+mierak@users.noreply.github.com> Date: Wed, 1 Jan 2025 21:55:35 +0100 Subject: [PATCH] fix: lsinfo not supporting playlists (#197) * fix: lsinfo not supporting playlists This properly supports the `playlist` entry in `lsinfo` MPD protocol command. The `playlist` as a key was ignored untill now because that behaviour is deprecated by mpd. The ignore was however done incorrectly and could result in incorrect parsing of MPD's response and sometimes even fail altogether. --- CHANGELOG.md | 1 + src/mpd/commands/lsinfo.rs | 104 +++++++++++++++++++++++++++++++++--- src/ui/panes/directories.rs | 24 +++++---- src/ui/panes/mod.rs | 13 ++--- 4 files changed, 118 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 276d7f5..4fb5b0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ All notable changes to this project will be documented in this file. - Styling not being applied to Bitrate and Crossfade props - Refactored and greatly simplified image backends - Potential infinite loop in lyrics indexing +- `lsinfo` parsing playlist entries incorrectly ## [0.7.0] - 2024-12-24 diff --git a/src/mpd/commands/lsinfo.rs b/src/mpd/commands/lsinfo.rs index 2ffba03..cb94ebd 100644 --- a/src/mpd/commands/lsinfo.rs +++ b/src/mpd/commands/lsinfo.rs @@ -6,12 +6,13 @@ use anyhow::Context; use derive_more::{AsMut, AsRef, Into, IntoIterator}; #[derive(Debug, Default, IntoIterator, AsRef, AsMut, Into)] -pub struct LsInfo(pub Vec); +pub struct LsInfo(pub Vec); #[derive(Debug, PartialEq, Eq)] -pub enum FileOrDir { +pub enum LsInfoEntry { Dir(Dir), File(Song), + Playlist(Playlist), } #[derive(Debug, Default, PartialEq, Eq)] @@ -23,6 +24,11 @@ pub struct Dir { pub last_modified: String, } +#[derive(Debug, Default, PartialEq, Eq)] +pub struct Playlist { + name: String, +} + impl FromMpd for Dir { fn next_internal(&mut self, key: &str, value: String) -> Result { match key { @@ -35,7 +41,16 @@ impl FromMpd for Dir { self.full_path = value; } "last-modified" => self.last_modified = value, - "playlist" => {} // ignore, deprecated + _ => return Ok(LineHandled::No { value }), + } + Ok(LineHandled::Yes) + } +} + +impl FromMpd for Playlist { + fn next_internal(&mut self, key: &str, value: String) -> Result { + match key { + "playlist" => self.name = value, _ => return Ok(LineHandled::No { value }), } Ok(LineHandled::Yes) @@ -45,10 +60,13 @@ impl FromMpd for Dir { impl FromMpd for LsInfo { fn next_internal(&mut self, key: &str, value: String) -> Result { if key == "file" { - self.0.push(FileOrDir::File(Song::default())); + self.0.push(LsInfoEntry::File(Song::default())); } if key == "directory" { - self.0.push(FileOrDir::Dir(Dir::default())); + self.0.push(LsInfoEntry::Dir(Dir::default())); + } + if key == "playlist" { + self.0.push(LsInfoEntry::Playlist(Playlist::default())); } match self.0.last_mut().context(anyhow!( @@ -56,8 +74,80 @@ impl FromMpd for LsInfo { key, value ))? { - FileOrDir::Dir(dir) => dir.next_internal(key, value), - FileOrDir::File(song) => song.next_internal(key, value), + LsInfoEntry::Dir(dir) => dir.next_internal(key, value), + LsInfoEntry::File(song) => song.next_internal(key, value), + LsInfoEntry::Playlist(playlist) => playlist.next_internal(key, value), + } + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use crate::mpd::commands::lsinfo::{Dir, LsInfoEntry, Playlist}; + + use super::{FromMpd, LsInfo}; + + #[test] + fn can_parse_playlist_entry() { + let input = r"playlist: autechre.m3u +Last-Modified: 2024-10-30T00:04:26Z +directory: .cue +Last-Modified: 2024-11-02T02:55:40Z +directory: .win +Last-Modified: 2024-09-15T19:39:47Z +directory: flac +Last-Modified: 2024-12-23T00:11:38Z +directory: wav +Last-Modified: 2024-08-12T03:03:40Z"; + + let mut result = LsInfo::default(); + for line in input.lines() { + let (key, value) = line.split_once(": ").unwrap(); + result + .next_internal(key.to_lowercase().as_str(), value.to_owned()) + .unwrap(); } + + let result = result.0; + assert_eq!(result.len(), 5); + assert_eq!( + result[0], + LsInfoEntry::Playlist(Playlist { + name: "autechre.m3u".to_owned(), + }) + ); + assert_eq!( + result[1], + LsInfoEntry::Dir(Dir { + path: ".cue".to_owned(), + full_path: ".cue".to_owned(), + last_modified: "2024-11-02T02:55:40Z".to_owned() + }) + ); + assert_eq!( + result[2], + LsInfoEntry::Dir(Dir { + path: ".win".to_owned(), + full_path: ".win".to_owned(), + last_modified: "2024-09-15T19:39:47Z".to_owned() + }) + ); + assert_eq!( + result[3], + LsInfoEntry::Dir(Dir { + path: "flac".to_owned(), + full_path: "flac".to_owned(), + last_modified: "2024-12-23T00:11:38Z".to_owned() + }) + ); + assert_eq!( + result[4], + LsInfoEntry::Dir(Dir { + path: "wav".to_owned(), + full_path: "wav".to_owned(), + last_modified: "2024-08-12T03:03:40Z".to_owned() + }) + ); } } diff --git a/src/ui/panes/directories.rs b/src/ui/panes/directories.rs index e65ca8e..fafe1b7 100644 --- a/src/ui/panes/directories.rs +++ b/src/ui/panes/directories.rs @@ -7,7 +7,7 @@ use crate::{ context::AppContext, mpd::{ client::Client, - commands::{lsinfo::FileOrDir, Song}, + commands::{lsinfo::LsInfoEntry, Song}, mpd_client::{Filter, FilterKind, MpdClient, Tag}, }, shared::{ext::mpd_client::MpdClientExt, key_event::KeyEvent, macros::status_info, mouse_event::MouseEvent}, @@ -65,12 +65,13 @@ impl DirectoriesPane { let new_current = client.lsinfo(Some(&next_path.join("/").to_string()))?; let res = new_current .into_iter() - .map(|v| match v { - FileOrDir::Dir(d) => DirOrSong::Dir { + .filter_map(|v| match v { + LsInfoEntry::Dir(d) => Some(DirOrSong::Dir { name: d.path, full_path: d.full_path, - }, - FileOrDir::File(s) => DirOrSong::Song(s), + }), + LsInfoEntry::File(s) => Some(DirOrSong::Song(s)), + LsInfoEntry::Playlist(_) => None, }) .sorted() .collect(); @@ -117,7 +118,7 @@ impl Pane for DirectoriesPane { let result = client .lsinfo(None)? .into_iter() - .map(Into::::into) + .filter_map(Into::>::into) .sorted() .collect::>(); Ok(MpdQueryResult::DirOrSong { @@ -143,7 +144,7 @@ impl Pane for DirectoriesPane { let result = client .lsinfo(None)? .into_iter() - .map(Into::::into) + .filter_map(Into::>::into) .sorted() .collect::>(); Ok(MpdQueryResult::DirOrSong { @@ -316,12 +317,13 @@ impl BrowserPane for DirectoriesPane { } .0 .into_iter() - .map(|v| match v { - FileOrDir::Dir(dir) => DirOrSong::Dir { + .filter_map(|v| match v { + LsInfoEntry::Dir(dir) => Some(DirOrSong::Dir { name: dir.path, full_path: dir.full_path, - }, - FileOrDir::File(song) => DirOrSong::Song(song), + }), + LsInfoEntry::File(song) => Some(DirOrSong::Song(song)), + LsInfoEntry::Playlist(_) => None, }) .sorted() .map(|v| v.to_list_item_simple(config)) diff --git a/src/ui/panes/mod.rs b/src/ui/panes/mod.rs index 764e9f5..cad94f4 100644 --- a/src/ui/panes/mod.rs +++ b/src/ui/panes/mod.rs @@ -160,7 +160,7 @@ pub(crate) mod browser { use crate::{ config::theme::SymbolsConfig, - mpd::commands::{lsinfo::FileOrDir, Song}, + mpd::commands::{lsinfo::LsInfoEntry, Song}, }; impl Song { @@ -296,13 +296,14 @@ pub(crate) mod browser { } } - impl From for DirOrSong { - fn from(value: FileOrDir) -> Self { + impl From for Option { + fn from(value: LsInfoEntry) -> Self { match value { - FileOrDir::Dir(crate::mpd::commands::lsinfo::Dir { path, full_path, .. }) => { - DirOrSong::Dir { name: path, full_path } + LsInfoEntry::Dir(crate::mpd::commands::lsinfo::Dir { path, full_path, .. }) => { + Some(DirOrSong::Dir { name: path, full_path }) } - FileOrDir::File(song) => DirOrSong::Song(song), + LsInfoEntry::File(song) => Some(DirOrSong::Song(song)), + LsInfoEntry::Playlist(_) => None, } } }