Skip to content

Commit

Permalink
fix: lsinfo not supporting playlists (#197)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
mierak authored Jan 1, 2025
1 parent af794bd commit 400d3b1
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
104 changes: 97 additions & 7 deletions src/mpd/commands/lsinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<FileOrDir>);
pub struct LsInfo(pub Vec<LsInfoEntry>);

#[derive(Debug, PartialEq, Eq)]
pub enum FileOrDir {
pub enum LsInfoEntry {
Dir(Dir),
File(Song),
Playlist(Playlist),
}

#[derive(Debug, Default, PartialEq, Eq)]
Expand All @@ -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<LineHandled, MpdError> {
match key {
Expand All @@ -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<LineHandled, MpdError> {
match key {
"playlist" => self.name = value,
_ => return Ok(LineHandled::No { value }),
}
Ok(LineHandled::Yes)
Expand All @@ -45,19 +60,94 @@ impl FromMpd for Dir {
impl FromMpd for LsInfo {
fn next_internal(&mut self, key: &str, value: String) -> Result<LineHandled, MpdError> {
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!(
"No element in accumulator while parsing LsInfo. Key '{}' Value :'{}'",
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()
})
);
}
}
24 changes: 13 additions & 11 deletions src/ui/panes/directories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -117,7 +118,7 @@ impl Pane for DirectoriesPane {
let result = client
.lsinfo(None)?
.into_iter()
.map(Into::<DirOrSong>::into)
.filter_map(Into::<Option<DirOrSong>>::into)
.sorted()
.collect::<Vec<_>>();
Ok(MpdQueryResult::DirOrSong {
Expand All @@ -143,7 +144,7 @@ impl Pane for DirectoriesPane {
let result = client
.lsinfo(None)?
.into_iter()
.map(Into::<DirOrSong>::into)
.filter_map(Into::<Option<DirOrSong>>::into)
.sorted()
.collect::<Vec<_>>();
Ok(MpdQueryResult::DirOrSong {
Expand Down Expand Up @@ -316,12 +317,13 @@ impl BrowserPane<DirOrSong> 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))
Expand Down
13 changes: 7 additions & 6 deletions src/ui/panes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -296,13 +296,14 @@ pub(crate) mod browser {
}
}

impl From<FileOrDir> for DirOrSong {
fn from(value: FileOrDir) -> Self {
impl From<LsInfoEntry> for Option<DirOrSong> {
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,
}
}
}
Expand Down

0 comments on commit 400d3b1

Please sign in to comment.