kimtore / pms

Practical Music Search is an interactive Vim-like console client for the Music Player Daemon.
https://ambientsound.github.io/pms/
MIT License
249 stars 23 forks source link

Add ability to set the playback mode #114

Closed christarazi closed 8 months ago

codecov-io commented 6 years ago

Codecov Report

Merging #114 into master will decrease coverage by 0.11%. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
- Coverage   52.06%   51.95%   -0.12%     
==========================================
  Files          52       52              
  Lines        1888     1892       +4     
==========================================
  Hits          983      983              
- Misses        879      883       +4     
  Partials       26       26
Impacted Files Coverage Δ
options/defaults.go 0% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8c9440a...a2204b5. Read the comment docs.

kimtore commented 6 years ago

Hi @christarazi, thank you for the PR.

I think it is convenient to support setting these parameters through options, I would like to see this functionality in PMS. However, there are a few issues I would like you to address before merging:

christarazi commented 6 years ago

There must be a null check on CurrentMpdClient, otherwise the client will crash when there is no MPD connection.

Fixed in 69fc22d. Adding the nil check at the top (before the switch) caused PMS to not load correctly on start up, for some reason. So I resorted to putting the retrieving and checking in each case statement. Please let me know if you have another way in mind.

The options should be updated when the server changes them, and they should always be in sync with the server settings.

Fixed in a2204b5

christarazi commented 6 years ago

Is this good to go @ambientsound?

kimtore commented 8 months ago

The design of this feature irks me, because I don't think the options should be so tightly coupled with MPD. Closing this five years after the fact. :man_shrugging: