Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.org
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ The library is implemented around a set of entities.
| *Name* | *Type* | *Fields* | *Description* |
|------------------+-----------+----------------------+---------------------------------------------------------|
| song | structure | name, album, file, … | |
| album | structure | name, artist | |
| album | structure | name, date, artist | |
| artist | structure | name | |
| genre | structure | name | |
| directory | structure | name, path | |
Expand Down
46 changes: 31 additions & 15 deletions libmpdel.el
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
(require 'tq)
(require 'cl-lib)
(require 'subr-x)
(require 'seq)


;;; Customization
Expand Down Expand Up @@ -171,6 +172,7 @@ message from the server.")
(:constructor libmpdel--album-create)
(:conc-name libmpdel--album-))
(name nil :read-only t)
(date nil :read-only t)
(artist nil :read-only t))

(cl-defstruct (libmpdel-song
Expand Down Expand Up @@ -352,9 +354,16 @@ If the SONG's name is nil, return the filename instead."
"Return the track number of SONG within its album."
(or (libmpdel--song-track song) ""))

(defun libmpdel-entity-date (song)
"Return the date of SONG."
(or (libmpdel--song-date song) ""))
(cl-defgeneric libmpdel-entity-date (entity)
"Return the date of ENTITY.")

(cl-defmethod libmpdel-entity-date ((album libmpdel-album))
"Return ALBUM's date."
(libmpdel--album-date album))

(cl-defmethod libmpdel-entity-date ((song libmpdel-song))
"Return SONG's date."
(libmpdel--song-date song))

(defun libmpdel-song-disc (song)
"Return the disc number of SONG within its album."
Expand Down Expand Up @@ -390,9 +399,7 @@ If the SONG's name is nil, return the filename instead."
:file (cdr (assq 'file song-data))
:performers (libmpdel--artists-create (libmpdel-entries song-data 'Performer))
:genres (libmpdel--genres-create (libmpdel-entries song-data 'Genre))
:album (libmpdel--album-create
:name (cdr (assq 'Album song-data))
:artist (libmpdel--artist-create :name (cdr (assq 'Artist song-data))))
:album (libmpdel--create-album-from-data song-data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems unrelated to "Add date field to albums". Can we keep that for another PR? I will better understand the goal this way and we can merge this one sooner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is technically unrelated to the field itself, but imo intimately related to the code: adding date fields to albums (and populating those fields with dates) means that the exact same album-creation-code would have to be duplicated in three separate places, to prevent that I factored said code into a function

(especially since in the next PR I'll add even more code to libmpdel--create-album-from-data, which would otherwise also have to be copied identically to three different places)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, thank you

:date (cdr (assq 'Date song-data))
:disc (cdr (assq 'Disc song-data))
:id (cdr (assq 'Id song-data))
Expand All @@ -402,6 +409,17 @@ If the SONG's name is nil, return the filename instead."
"Return a list of songs from DATA, a server's response."
(mapcar #'libmpdel--create-song-from-data (libmpdel-group-data data)))

(defun libmpdel--create-album-from-data (song-data)
"Return an album from SONG-DATA, a server's response."
(libmpdel--album-create
:name (cdr (assq 'Album song-data))
:date (cdr (assq 'Date song-data))
:artist (libmpdel--artist-create :name (cdr (assq 'Artist song-data)))))

(defun libmpdel--create-albums-from-data (data)
"Return a list of albums from DATA, a server's response."
(mapcar #'libmpdel--create-album-from-data (libmpdel-group-data data)))

(defun libmpdel-current-playlist-p (entity)
"Return non-nil if ENTITY is the current playlist."
(eq entity 'current-playlist))
Expand Down Expand Up @@ -1003,11 +1021,9 @@ If HANDLER is nil, ignore response."
"list album"
(lambda (data)
(funcall function
(mapcar
(lambda (album-name)
(libmpdel--album-create :name album-name
:artist libmpdel--unknown-artist))
(libmpdel-sorted-entries data 'Album))))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before we merge this PR, it would be good to have a PR ready to be merged that introduce sorting back into the UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on my tests, the output of list album is already alphabetically sorted, but that is not documented - is that good enough?

(seq-uniq
(libmpdel--create-albums-from-data data)
#'libmpdel-equal)))))

(cl-defmethod libmpdel-list ((_entity (eql genres)) function)
"Call FUNCTION with all genres as parameter."
Expand All @@ -1033,12 +1049,12 @@ If HANDLER is nil, ignore response."
(cl-defmethod libmpdel-list ((artist libmpdel-artist) function)
"Call FUNCTION with all albums of ARTIST as parameter."
(libmpdel-send-command
`("list album %s" ,(libmpdel-entity-to-criteria artist))
`("find %s sort AlbumSort" ,(libmpdel-entity-to-criteria artist))
(lambda (data)
(funcall function
(mapcar
(lambda (album-name) (libmpdel--album-create :name album-name :artist artist))
(libmpdel-sorted-entries data 'Album))))))
(seq-uniq
(libmpdel--create-albums-from-data data)
#'libmpdel-equal)))))

(cl-defgeneric libmpdel-list-songs (entity function)
"Call FUNCTION with all songs of ENTITY."
Expand Down
3 changes: 3 additions & 0 deletions test/libmpdel-test.el
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
(let ((song (libmpdel--create-song-from-data
'((Title . "The song")
(file . "foo/song.ogg")
(Date . "1952-08-29")
(Album . "The Album")
(Performer . "The Violinist")
(Performer . "The Pianist")
Expand All @@ -125,6 +126,8 @@
(should (equal "The song" (libmpdel-entity-name song)))
(should (equal "foo/song.ogg" (libmpdel-song-file song)))
(should (equal (list "The Genre") (mapcar #'libmpdel-entity-name (libmpdel-genres song))))
(should (equal "1952-08-29" (libmpdel-entity-date song)))
(should (equal "1952-08-29" (libmpdel-entity-date (libmpdel-album song))))
(should (equal "The Album" (libmpdel-entity-name (libmpdel-album song))))
(should (equal (list "The Violinist" "The Pianist" "The Singer")
(mapcar #'libmpdel-entity-name (libmpdel-performers song))))
Expand Down