hannesm / jackline

minimalistic secure XMPP client in OCaml
BSD 2-Clause "Simplified" License
251 stars 20 forks source link

Decomission notification.state #118

Closed hannesm closed 8 years ago

hannesm commented 8 years ago

since the callback is strictly more general. Also, fuzzing around with files is something I dislike (after all, this is mutable state ;).

any opinion @sg2342? anyone else (@schoeke @cfcs @posiputt maybe) whose scripts rely on the notification.state file being present? maybe providing an example shell script which writes out that file would be sufficient?

infinity0 commented 8 years ago

To elaborate on what I said on IRC: the best behaviour for notification_callback is to fire it every time a message arrives, and that's a separate issue from clearing the notification. I agree that notification.state is not necessary as long as you pass the contents to the callback, although it can make it more convenient for people.

The reason why it's best to do this for every message, is because that's the behaviour for normal IM clients. It's useful to know that I got a message 1s ago, regardless of whether there's an unread message from 1 hour ago, or not.

OTOH, there should be a separate mechanism for clearing the notification when the user later focuses on jackline / temporarily suppressing the default behaviour of "firing notification_callback for every message", for the duration that the user is already focused on jackline. This separate mechanism is something that jackline can't predict, e.g. if the terminal/GUI window has focus, and therefore it's best not to mix it up with the logic of "when I should fire notification_callback" that is internal to jackline. (Perhaps I should open another issue for this?)

hannesm commented 8 years ago

(since 9d76f38a73ba9ae42b7eebd8fedc52fce0983f92 the behaviour is to fire the callback (and notification.state modification) on every received message (instead of only if the state actually changes); previous behaviour also included that if the currently focused contact sends a message, the callback/file change did never happen).

The callback is any program, and gets two arguments passed: (a) your full jabber id and (b) the new state (one of "quit", "disconnected", "connected", "disconnected_notifications", "connected_notifications").

hannesm commented 8 years ago

oh, also the suppress something on focus / no focus / screen lock active / what you say -- this is best done outside of jackline in your favorite scripting language and window manager. I won't add any such 'suppress notification' mechanism to jackline.

infinity0 commented 8 years ago

But jackline needs to provide some information to the external suppression mechanism. For example, these 4 scenarios need to be responded to differently, in a sane UI:

It would be fine to pass this information to notification_callback as arguments. Perhaps other information is necessary too, need some time to think about that.

infinity0 commented 8 years ago

Thinking about the problem further, a UI generally wants to communicate these things:

  1. when a message is received (a point-event, e.g. play a sound)
  2. whether there are unread messages (an ongoing state, e.g. show something in a different colour)

(1) is easily served by notification_callback but (2) is harder. We can decompose it:

In other words, whatever decides (2) needs to know both when jackline has focus on each contact (x), and when jackline the app has focus in the UI. So jackline either needs to calculate (2), or provide to the external mechanism that calculates (2), the information (x) AND (the contact that is relevant for each notification).

cfcs commented 8 years ago

I'm using this atm: https://gist.github.com/christianpanton/e3d84a25ae98e71e1d85

posiputt commented 8 years ago

I haven't been following this topic all that close. is notification_callback already available? if so: how would I go about accessing it? I could modify my notification script accordingly, I guess.

Update: never mind, it's all in the README.md.

hannesm commented 8 years ago

@posiputt it is available.. by putting (notification_callback (/my/shell.sh)) into config.sexo, just before the final closing ), the mentioned script -- shell.sh will be executed (with me@jabber.com/jackline (or similar) as first argument, connected_notifications (and other values) as second argument)... both the notifications.state and the callback are there at the moment..

posiputt commented 8 years ago

@hannesm thanks. I already tried it out. I noticed that apparently jackline doesn't run the callback script when exiting properly. That would be nice though.

infinity0 commented 8 years ago

raised this on another medium, copying here so it doesn't get lost - regarding my previous comment, it is not necessary for jackline to emit the exact uid of a contact, it can pick a random string per contact that is constant for the lifetime of the jackline process. This can help prevent information leaks in case the notification callback is written badly. (It does need to be constant for the lifetime of the process however, in order for us to be able to calculate the correct "there exist some unread messages" semantics as mentioned in (2) above.)

infinity0 commented 8 years ago

wip at infinity0/jackline@notify-finer comments welcome

hannesm commented 8 years ago

if you can open a PR and describe briefly what you do/did, I'd be happy to review..

hannesm commented 8 years ago

there's no longer a notification.state... but two arguments to jackline (one for input whether jackline has focus, one for output of notifications)... hope people will be able to port their notification scripts.. thx @infinity0!

hannesm commented 8 years ago

@infinity0 now that this is merged, could you remind me why --fd-gui and notification_callback is needed? shouldn't one of those be sufficient?

infinity0 commented 8 years ago

You mean fd-nfy? Yes, one of those is sufficient. I added fd-nfy just for convenience, to deliver all notifications to one single reader. (With notification_callback you need to write extra logic to find that single reader.)

hannesm commented 8 years ago

ah right, thanks. and yes, I meant --fd-nfy...