Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[widgets.mpd] Add fields ${Artists} and ${Genres} #117

Merged
merged 7 commits into from
Dec 10, 2023

Conversation

toniz4
Copy link
Contributor

@toniz4 toniz4 commented Dec 6, 2023

Resolves GH-116

Adds new return values to widgets.mpd that concatenate all artists and generes when there's more than one. For tracks with vorbis metadata.

I've also updated the docs to reflect those changes.

Copy link
Member

@McSinyx McSinyx left a comment

Choose a reason for hiding this comment

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

The patchset looks good to me, aside from some minor formatting issues. After you're done resolving them, could you please test if both Artists and Genres are working?

docs/source/widgets.rst Outdated Show resolved Hide resolved
docs/source/widgets.rst Outdated Show resolved Hide resolved
widgets/mpd_all.lua Outdated Show resolved Hide resolved
widgets/mpd_all.lua Outdated Show resolved Hide resolved
@McSinyx McSinyx changed the title [widgets.mpd] Add fields ${Artists} and ${Generes} [widgets.mpd] Add fields ${Artists} and ${Genres} Dec 7, 2023
@toniz4
Copy link
Contributor Author

toniz4 commented Dec 7, 2023

The patchset looks good to me, aside from some minor formatting issues. After you're done resolving them, could you please test if both Artists and Genres are working?

Genres was broken before because of the typo, but now It's working fine:

Here's the output:
image

The update function:

local update = function (widget, args)
   if args["{state}"] == "Stop" then
      widget.visible = false
      return ''
   else
      widget.visible = true
      return {
          ('Artists: %s Track: %s Genres: %s'):format(args["{Artists}"], args["{Title}"], args["{Genres}"]),
         args["{state}"]
      }
   end
end

And what mpd is returning:

$ printf 'password ""\nstatus\ncurrentsong\nclose \n' | curl --connect-timeout 1 -fsm 3 telnet://127.0.0.1:6600
OK MPD 0.23.5
ACK [3@0] {password} incorrect password
volume: 100
repeat: 0
random: 0
single: 0
consume: 0
partition: default
playlist: 1537
playlistlength: 192
mixrampdb: 0
state: play
song: 182
songid: 1556
time: 284:321
elapsed: 284.284
bitrate: 1221
duration: 320.625
audio: 44100:16:2
nextsong: 183
nextsongid: 1557
OK
file: Parannoul/Parannoul - To See the Next Part of the Dream/01 - Beautiful World.flac
Last-Modified: 2023-02-20T06:28:07Z
Format: 44100:16:2
Title: Beautiful World
Artist: Parannoul
Album: To See the Next Part of the Dream
AlbumArtist: Parannoul
Track: 1
Disc: 1
Genre: Asian Music
Genre: K-Pop
Date: 2021-04-19
Time: 321
duration: 320.625
Pos: 182
Id: 1556
OK

More than one artist it's working as expected too:

image

$ printf 'password ""\nstatus\ncurrentsong\nclose \n' | curl --connect-timeout 1 -fsm 3 telnet://127.0.0.1:6600
OK MPD 0.23.5
ACK [3@0] {password} incorrect password
volume: 100
repeat: 0
random: 0
single: 0
consume: 0
partition: default
playlist: 1740
playlistlength: 202
mixrampdb: 0
state: play
song: 193
songid: 1759
time: 75:152
elapsed: 74.878
bitrate: 507
duration: 152.000
audio: 44100:16:2
nextsong: 194
nextsongid: 1760
OK
file: ceo@business.net - incentivize unpaid overtime/02 - buttercup.flac
Last-Modified: 2023-02-19T21:45:43Z
Format: 44100:16:2
Title: buttercup
Artist: ceo@business.net
Artist: lentra
Album: incentivize unpaid overtime
AlbumArtist: ceo@business.net
AlbumArtist: lentra
Track: 2
Disc: 1
Genre: Rap/Hip Hop
Date: 2020-02-21
Time: 152
duration: 152.000
Pos: 193
Id: 1759
OK

Here I couldn't find any tracks that have multiples of other tags, not being genre, artist or albumArtist. So just concatenating Artist and Genre it's fine IMO.

Copy link
Member

@McSinyx McSinyx left a comment

Choose a reason for hiding this comment

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

Thanks, I will merge and tag a new release this weekend.

@McSinyx McSinyx merged commit 969d942 into vicious-widgets:master Dec 10, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[vicious.widgets.mpd] Only the last artist is being returned in "{Artist}".
2 participants