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

Make Instant-Lyrics app-agnostic #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nicolas-guichard
Copy link

Why restrict to Spotify lyrics? Instant-Lyrics is already using MPRIS, so it can be aware of music apps running. This is what my changes do:

  • Instant-Lyrics can now list running music apps and update the AppIndicator accordingly
  • It will do so upon each DBus event related to music apps (I couldn't figure a way to reliably rebuild the menu upon opening, it may list closed apps until following DBus event)

I also corrected an encoding problem with accents.

This shows a summary of the changes:
capture du 2017-04-01 17-40-52

@bhrigu123
Copy link
Owner

allow me some time on this.

'(KHTML, like Gecko) Chrome/23.0.1271.64 Safari/537.11',
'Accept-Language': 'en-US,en;q=0.8',
hdr = {'User-Agent': 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0',
'Accept-Encoding': 'gzip, deflate, br',
Copy link

Choose a reason for hiding this comment

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

On my side the request is not decoded correctly when enabling brötli.
Everything works fine again when changing this line to:
'Accept-Encoding': 'gzip, deflate',

@simonbru
Copy link

simonbru commented Nov 6, 2017

This feature works well on my side after the fix I mentioned above.
One minor issue: the app list does not update right away after closing a media player. However it will get updated after any other action triggering a signal from /org/mpris/MediaPlayer2 so this is probably good enough.

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.

3 participants