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

[modules/playerctl]: use playerctl -f and add playerctl.args #793

Merged
merged 2 commits into from
Jun 11, 2021

Conversation

ouuan
Copy link
Contributor

@ouuan ouuan commented Jun 11, 2021

  1. Use playerctl -f to format, which is more powerful. This also fixes playerctl not displaying all information #767, which is caused by missing a few fields of the metadata.
  2. Add playerctl.args, so that users can choose a specific player, etc.
  3. Display nothing when there's no running player.

This is a breaking change. Users need to change {title} to {{title}}.

I'm not sure how should I handle a breaking change. Should I add a new parameter like playerctl.playerctl_format, or replace {artist} with {{artist}}, or just break it?

…args`

1. Use `playerctl -f` to format, which is more powerful. This also fixes
   tobi-wan-kenobi#767, which is caused by missing a few fields of the metadata.
2. Add `playerctl.args`, so that users can choose a specific player,
   etc.
3. Display nothing when there's no running player.

This is a breaking change. Users need to change `{title}` to
`{{title}}`.
@ouuan ouuan changed the title [modules/playerctl]: BREAKING: use playerctl -f and add playerctl.args [modules/playerctl]: use playerctl -f and add playerctl.args Jun 11, 2021
@gkeep
Copy link
Contributor

gkeep commented Jun 11, 2021

Looks good to me!

I think the best option is to just break it.

The change to parameters will be noted in the changelog for a future release. Plus users who didn't notice the change will notice it when the module doesn't work after the update.

@tobi-wan-kenobi tobi-wan-kenobi self-assigned this Jun 11, 2021
@tobi-wan-kenobi
Copy link
Owner

The change to parameters will be noted in the changelog for a future release

I am grateful for that vote of confidence, now I just need to jot that down so I don't forget :P

I agree, I would also just go ahead and break the interface. If somebody complains, we can always re-introduce a compatibility layer. But generally, I would like to avoid code growth because we try to stay backwards compatible always.

@tobi-wan-kenobi
Copy link
Owner

  • the diff looks good to me, so in it goes. Thanks a lot for the contribution!

@tobi-wan-kenobi tobi-wan-kenobi merged commit ec71d7f into tobi-wan-kenobi:main Jun 11, 2021
@ouuan ouuan deleted the playerctl branch June 11, 2021 11:31
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.

playerctl not displaying all information
3 participants