Ring terminal bell on notification

[maximbaz]

This simple change makes terminal ring a bell when sending a notification.

There is typically two things that can happen:

• An audible sound (can be disabled in terminal config)
• An urgency hint (mark window as needs attention)

The second item is my personal motivation for this PR. It makes of a very nice integration with Desktop Environment, when I not only get a notification, but also i3/sway can highlight which workspace sent the notification, and not only that, but there are common tools that can bring you directly to the window that requires attention.

Signal Desktop also sets this urgency hint on their window on notification (which due to electron issue is broken on Wayland, so scli will become superior 🙌 ).

If terminal is in focus, the urgency hint is basically a noop (but not the sound, if user has a sound configured).

I thought of maybe introducing a separate configuration for this, but then decided against this - notification and urgency hint often come hand-in-hand in other apps, and if user wants to disable them, they likely disable both.

Let me know what you think!

[exquo]

Nice idea! When run inside tmux it conveniently highlights the tab.

It would be ideal to set the urgency hint without actually ringing the bell (even for the users who have the speaker beep enabled, perhaps unbeknownst to them). I've looked around, but haven't found anything better than passing the BEL character to the terminal.

However, we should be apprehensive about using print() while urwid's event loop is running: urwid alone should write anything to the terminal. This actually does produce an issue here, although it is intermittent and I can't reliably reproduce it in scli: when getting a notification the whole screen output might get shifted up, leaving traces of the old output which do not get overwritten. Scli modifies a few widgets on getting a new message, and the order in which this happen might determine whether this artifact occurs.

This minimal code (based on an example from urwid tutorial) shows it reliably:

import urwid
import time

def keypress(key):
    txt.set_text(' '.join((txt.text, repr(key))))
    #time.sleep(1)
    print('\a')

txt = urwid.Text("Hello World")
fill = urwid.Filler(txt)
loop = urwid.MainLoop(fill, unhandled_input=keypress)
loop.run()

It sends a BEL to the output on any keypress. On the first keypress the output gets shifted up one line. Removing a newline at the end with

print('\a', end='')

seems to fix it.

It would be safer to pass the BEL signal to urwid's display module more directly, but I have not seen anything like that among its interface methods. So I guess we can settle for the print() statement.

I think we can add a new config option to scli to disable this (e.g. --notification-no-bell) so that folks can have a desktop notification w/o an urgency hint (and maybe even vice versa).

BTW, I've also considered modifying the default --notification-command to something like printf '\a' && notify-send .. so that people could modify it directly without an additional option. But scli currently allows only a single executable to be called in NOTIFICATION COMMAND (mostly for security reasons).

I can make the required changes, but feel free to add / change anything in the PR. Thanks again for the suggestion!

[maximbaz]

It would be ideal to set the urgency hint without actually ringing the bell (even for the users who have the speaker beep enabled, perhaps unbeknownst to them). I've looked around, but haven't found anything better than passing the BEL character to the terminal.

Indeed! There are ways to do it like that in some specific DEs (e.g. I'm on sway and there's a way to make window urgent without using BEL, just knowing its PID), but I figured BEL is very cross-platform way to bring the desired functionality to as many people as wanted.

print('\a', end='')

Ohh that is definitely an oversight on my side, thanks for pointing it out! Your explanation and example makes sense too.

BTW, I've also considered modifying the default --notification-command to something like printf '\a' && notify-send .. so that people could modify it directly without an additional option.

This is a clever idea! To overcome the "single executable" restriction I tried notification-command = /bin/sh -c 'printf "\a" && notify-send "%s" "New Signal message"' but curiously I'm not receiving the bell, but I do receive notification 🤔

I think we can add a new config option to scli to disable this

If you want an option, I'll make one 👍

I can make the required changes, but feel free to add / change anything in the PR. Thanks again for the suggestion!

I will take care of it 😉

[maximbaz]

I think we can add a new config option to scli to disable this (e.g. --notification-no-bell) so that folks can have a desktop notification w/o an urgency hint ---> (and maybe even vice versa) <---.

To support the above I moved the bell higher in the function, let me know if that is not desired!

Or feel free to take over the PR if it's simpler!

[maximbaz]

I've been using this for a couple of days, initially I was afraid that it does break the interface, but I managed to confirm that it was instead caused by emojis (https://github.com/isamert/scli/issues/48#issuecomment-1025221994) and reproduces even on main branch. I couldn't get the interface broken with usual english text. So, hopefully things look good here 😁

[exquo]

I've made a few changes:

• Moved the bell config-check below the check for general notifications (like in your original commit) This effectively makes it a "sub"-option of --enable-notification, which enables both desktop notification and the bell. Like you said, most people who would want to disable one, would probably disable both. But we still leave them an option to enable just one of them.
• Removed the 'toggle' function. The users probably won't be changing this setting while scli is running. More likely, they'll set it in the config once and for all. And there is a toggleNotifications that will disable all notifications, including the bell.
• Made the bell enabled by default, if the desktop notifications are enabled.
• Mentioned "urgency hint" in help. Otherwise, folks are likely to see this 'bell ringing' as frivolous :). As discussed above, we don't really need the beep, but a side effect of brining attention to the window.

Appreciate your work on this! Feel free to push to this branch if you think of any additional changes - I'll just squash the commits before merging.

[maximbaz]

I agree with everything and I like it in the current state 👍

[exquo]

P.S. interesting how a one-line code change can be so un-straightforward :)