search

osa1 / tiny

A terminal IRC client
MIT License
1.01k stars 59 forks source link

Missing desktop notification features #72

Open meain opened 6 years ago

meain commented 6 years ago

I was thinking about implementing desktop notifications using rust-notify.

I need your opinion before finalising some decisions.

I was planning to add the notification code in messaging.rs, and will work more or less similar to adding a message to messaging area on new message.

Also probably add a config file option with options like [off, mentions, messages, all] This can be used by default and also maybe provide a command like /notifications <option> to change it on a per channel basis.

osa1 commented 6 years ago

I've been meaning to do this myself for a long time, so thanks for this.

I think adding it to messaging.rs makes sense, we can check flags in add_privmsg() and decide whether to show a desktop notification or not easily.

The flags and /notifications command also make sense.

One thing we should decide is whether to show notifications when the terminal window has focus. I had added FocusLost and FocusGained events to term_input for this purpose, but had not had the change to use them. Some other clients (e.g. konversation) only shows notifications when the window does not have focus, and I like that feature. We can implement a similar thing. (with the caveat that it won't work in tmux)

We can of course leave it as a future work.

meain commented 6 years ago

I tried out a few stuff and it seems like Tab in tabbed.rs is the best option to implement the code. We could still do something of this sort but in tabbed:

how about this: in src/tui/mod.rs we add one more line to each add_xyz_msg method, something like: self.notifier.add_privmsg_notification(target, msg)

Implementing it in MessagingUI will have issues with what tab it is, like server_name, channel_name etc.

Implementing it in TUI (as we talked) will make it harder to get nick and maintaining separate notification options for different tabs.

sounds good, @osa1 ?

osa1 commented 6 years ago

Implementing it in TUI (as we talked) will make it harder to get nick

Why do you need to know about the nick to show notifications?

You're right that if we want separate notification options for different tabs then it should be in Tab.

meain commented 6 years ago

Unless we know the nick we cannot differentiate between messages got from the server and typed by the user. At least that is what I understand.

osa1 commented 6 years ago

Just landed the initial support. Two features missing:

eoli3n commented 3 years ago

Just tried it, it seems not working with /notify mentions

osa1 commented 3 years ago

What is not working with /notify mentions?

eoli3n commented 3 years ago

Desktop notifications, is that not the name of that issue ? In the README

/notify [off|mentions|messages]: Enable and disable desktop notifications. Running this command in a server tab applies it to all channels of that server. You can check your notify state in the status line.

osa1 commented 3 years ago

I think you're having the same problem described in my comment here. Could you check it please? I will update tiny to print a warning like "Desktop notifications are not enabled on this build, please see ... for building with desktop notifications enabled or get install from a pre-built library" or something like that when the build doesn't support desktop notifications.

osa1 commented 3 years ago

We now print a warning when a user uses /notify but the build doesn't support desktop notifications.