search

ravachol / kew

A command-line music player
GNU General Public License v2.0
567 stars 21 forks source link

[Feature request] notification on song change #108

Closed werdahias closed 6 months ago

werdahias commented 7 months ago

It'd be great if (optionally) kew could send a notification via libnotify to indicate the next title so the user knows which song comes next even if they don't have the terminal with kew focused.

ravachol commented 7 months ago

Yes, absolutely.

xplshn commented 7 months ago

Great. Could this be an optional feature? And the library statically linked? So that users with minimal setups don't have to install libnotify?

ravachol commented 7 months ago

It would be optional. but I'm actually debating whetever we want this or not. Having to deal with mpris is bad enough. I would prefer not to add more external dependencies.

xplshn commented 7 months ago

I didn't know you added support for it. I guess good for those who use MPRIS, personally I keep tabs on what packages my system has, I try to cut the fat, is the new MPRIS support optional? Or is it always built with it?

ravachol commented 7 months ago

mpris support doesn't add any new packages so don't worry about it, but supporting it adds some complexity to the development of the app, because we need to test all the mpris widgets not just kew. The mpris stuff is sort of done now though.

werdahias commented 7 months ago

the dynamically linked kew has just 807 kb on my system, so I don't see how libnotify support would raise the size much

xplshn commented 7 months ago

the dynamically linked kew has just 807 kb on my system, so I don't see how libnotify support would raise the size much

For me its not about size, I want to know what packages my system has, and where their content's are located, so for me its a big deal, I only have 251 packages in my system right now. I don't like that number going up.

xplshn commented 7 months ago

But ofc this is @ravachol's project, and I am not the only one who uses it, its gained some traction, I would just like to suggest that mpris support is made optional (Not that you need for kew to work, optional as in: Unless you have the mpris-dev package installed, mpris support will not be baked into kew)

ravachol commented 7 months ago

I'd rather not introduce different types of builds of kew, and have to test through all those. libnotify would be optional as in either on or off as a user setting, default off. Not as a linked or not library.

mpris is supported through glib2.0 which is needed anyway.

This is what it says in the 'contributing' document:

"The goal of kew is to provide a quick and easy way for people to listen to music with the absolute minimum of inconvenience. It's a small app, limited in scope and it shouldn't be everything to all people. It should continue to be a very light weight app. For instance, it's not imagined as a software for dj'ing or as a busy music file manager with all the features.

We want to keep the codebase easy to manage and free of bloat, so might reject a feature out of that reason only."

I'm trying to stay true to that vision. libnotify is a tiny library though so might not be that big of a deal, I dunno. I haven't looked at it closely yet.

But @werdahias did you request a notification ahead of a song being played or to be notified when the song switches?

xplshn commented 7 months ago

What about using notify-send if it is available? Or herbe if notify-send is not available? Won't that be easier, shorter & more compatible?

xplshn commented 7 months ago

So, sytems without dbus, which might have herbe & tiramisu can use that feature as well, and systems that are freedesktop.org compliant will benefit from having their notification's daemon receive the notifications via notify-send...

ravachol commented 7 months ago

using notify-send instead of a lib, sounds good to me.

xplshn commented 7 months ago

I am thinking that it could be done this way:

Use command -v to check if notify-send is available, if not, use command -v to check if herbe is available.
         \_______|_______/                                                                                       \_______|_______/
                        v                                                                                                                    v
           Use notify-send                                                                                                   Use herbe

  https://github.com/dudik/herbe, or maybe dunst too?
xplshn commented 6 months ago

@werdahias You can mark this as closed now, @ravachol added the feature for user's that have/use "notify-send".