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

Print positions in the search playlist function #2109

Merged

Conversation

jcorporation
Copy link
Member

For simple listing this can be done by the client, but for the search case the client can not determine the correct position.

I hope this does not break existing client implementations.

@jcorporation jcorporation changed the title Print positions in the list playlists function. Print positions in the list playlist functions. Sep 1, 2024
@MaxKellermann
Copy link
Member

If no detail was requested, MPD shouldn't ever print anything other than the URI.
If detail was requested, the position should only be printed if it is not redundant information, for example if this is a search result - the thing you're aiming at. Adding overhead to all response is useless.
(I consider this a mistake in the "playlistinfo" response - the "Pos" is always printed for no reason; but we can't remove it now.)

@jcorporation jcorporation force-pushed the searchplaylist_pos branch 2 times, most recently from af7d532 to 43ff2df Compare September 27, 2024 17:03
@jcorporation
Copy link
Member Author

Reworked to print the Pos only for search playlist command.

@jcorporation jcorporation changed the title Print positions in the list playlist functions. Print positions in the search playlist function Sep 27, 2024
@MaxKellermann
Copy link
Member

But it's still printed if no detail was requested

@jcorporation
Copy link
Member Author

Fixed

@MaxKellermann MaxKellermann merged commit 98bc632 into MusicPlayerDaemon:master Oct 26, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants