feat: notify on errors if not started from terminal

[robertwidfen]

See #663

One reasons for this: When starting from a key binding
the user will not see logged errors or warnings. Thus
we also ignore the notify config.

[RobertMueller2]

how about adding a logging component? Collect log messages in the background in a ring buffer (next to printing them to stderr), and have a button that is basically an error counter or something, once you click it, it opens a popup that displays the ring buffer. That way users could review any config errors even if they haven't started from terminal.

Notification only displays important stuff, like it is now.

[robertwidfen]

This sits on top of #698 now which I split from the initial code here.

Errors are now logged now and open a notification even if notify = false is set but not when started from terminal.

I dare adding a button - we also have no config UI and I hope our users can use a terminal. 😬

[RobertMueller2]

I'm not a fan of bypassing disable-notifications. I understand why, but I don't think we should. If a button is not an option, I suggest we display a message box dialog if notifications are disabled.

[RobertMueller2]

Well, let's get this merged for now. Can you please add a remark to disable-notifications that the setting may be ignored in case config errors can't be reported via terminal?

In the long run though, I think we should

• collect messages in a ring buffer (I don't know how big, maybe 1000 entries max)
• allow users to display them in a list dialog
• figure out a clever way to make users aware that there were config related messages and they need to open the dialog to show them
• add an accessible non-button way to open the dialog ;)
• perhaps even allow different log levels (info/warning/debug/trace)

I believe this approach is even user-friendlier because users would not have to figure out what command line bits they need to start Satty from terminal. And it works if users do not have a notification daemon -- rare, but not impossible.

[robertwidfen]

IMHO 42 should be sufficient for the history length.

I have an idea that solves everything - we create a red text annotations for the messages - this way the user gets it right into the face, can access the text and gets annoyed to fix the issues. 🤣

[RobertMueller2]

I think we need to consider stderr rather than stdout, because that's what we write to.

And I think this checks is_terminal too often. E.g. now I don't get a notification for a successful save. Do we really want to move that kind of stuff to the terminal output?

[robertwidfen]

And I think this checks is_terminal too often. E.g. now I don't get a notification for a successful save.

Hmm this is odd, why should it swallow the save-notification?

Do we really want to move that kind of stuff to the terminal output?

I like it during testing and it does no harm IMHO.

[robertwidfen]

Personally, I prefer toasts for most UI messages (warnings, errors, and success states) because they appear right where the user is looking. The only downside is that toasts do not leave a history like notifications, which could be handy for checking where an image was saved etc. I'm still undecided on the best balance here.

[RobertMueller2]

Personally, I prefer toasts for most UI messages (warnings, errors, and success states) because they appear right where the user is looking. The only downside is that toasts do not leave a history like notifications, which could be handy for checking where an image was saved etc. I'm still undecided on the best balance here.

We don't have to decide that now. #704, if implemented, may offer a history, within Satty. Notifications can remain the default for now. But if a user chooses to disable them via config, we can use toasts for errors that we can recover from, and a message box for errors that are fatal -- fatal errors are the only location where I'd personally see a use of an is_terminal check.

The lack of a history is a deliberate user choice. We don't know why a user makes it, but e.g. not having a notification daemon may be one of them, in that case the user would get nothing at all if we send a notification. I'd rather respect the user choice than selectively ignore it. In a way that is very difficult to predict from a user perspective.