[RobertMueller2]

This is a different approach to what #664 solves. Uses adwaita toast if notifications are disabled. Separate main loop is used display message for severe errors which happen before GTK is initialized. I believe this is better because it does not ignore disable-notifications.

• replace all remaining eprintln!s, but for this to work we need to do something useful if notifications are disabled and gtk has not been initialized yet
• LogSeverity::Urgent is currently unused
• revisit levels and severities - some revisited as per discussion below, anything else can go into followup issues and PRs
• perhaps consider 1 or 2 convenience functions, and/or reduce code duplication. But maybe also get rid of LogSeverity for all messages, but use a distinct function for notification worthy messages instead
• make install tested locally
• flatpak tested, but this required some change of the metadata (launchable, categories), otherwise my local flatpak-build would fail helper removed again
• nix develop .. tested at some point, but not with the last stuff helper removed again
• since --profile-startup is now behind debug log level, we can make that an alias for loglevel debug and simplify the code slightly
• fill out changelog

This should open the possibility for a log buffer and potential dialog to display it on demand (#704) but I'm not doing that here.

I'm also removing draft now because the remaining stuff would not be a lot anymore.

updated disclaimer on AI generated code

I had claude help me with the satty-fatal-error-dialog helper binary main function because I felt like outsourcing that part. The idea as such was my own after dealing with zenity. helper removed again

Breaking Section

Introduces a new feature, "fatal-error-dialog" which is activated by default. This installs a helper binary (satty-fatal-error-dialog) which can be used to display error messages before satty's config has been read or satty's own gtk has been initialized. satty-fatal-error-dialog does not depend on gl or epoxy so should work in more scenarios.

However, if you always read the terminal then this might be annoying, so the behaviour can be disabled by turning default-features off.

--profile-startup was changed so it sets level debug. The logging as such is no longer conditional. This way it'll always be available via log viewer (#785).

[robertwidfen]

I must admit that I like toasts - I even prefer them in comparison to notifications as they occur where I am looking anyway.

Adding log_warn() and log_error() would allow us to automatically prepend an ⚠️ or ❌. This would make warnings and errors much more visible and the locations where errors happen easier to grep in the code. Further, I would not auto close warnings and errors. I added log_error() in #664 to set the priority to keep the notification open.

[RobertMueller2]

I must admit that I like toasts - I even prefer them in comparison to notifications as they occur where I am looking anyway.

Adding log_warn() and log_error() would allow us to automatically prepend an ⚠️ or ❌. This would make warnings and errors much more visible and the locations where errors happen easier to grep in the code. Further, I would not auto close warnings and errors. I added log_error() in #664 to set the priority to keep the notification open.

yes, that's a good point.

[RobertMueller2]

Changed to draft, will probably take a few more days.

[RobertMueller2]

Almost done IMO, I think a second pass to levels and (perhaps) severities can be done whenever anyone sees fit.

Still need to figure out if/where we need to add Zenity runtime dep.

[RobertMueller2]

Right, so i tossed out the native-dialog again. We would have had to add zenity to the flatpak. Flatpak is arguably one of the worst places where you could rely on errors just showing up on stderr.

But adding such a package to it creates a moving target, because we need to include it by download url and hash.

So instead, I added a helper binary which just spawns a gtk dialog, which can be used before config is read and before gtk is initialized. With epoxy and config errors and gl and all that there are some errors that might just be swallowed silently.

For anyone who's annoyed by the extra popup because they read the terminal anyway, this is behind a feature that can be disabled.

I think I still need to test if the flatpak still works.

[robertwidfen]

IMHO config reading error is the only one where we actually would need the extra binary satty-fatal-error-dialog, but IMHO we should not abort here, but just report it and use the defaults. If GTK fails also satty-fatal-error-dialog will fail.

That way we could avoid having an external binary. 🤔

[robertwidfen]

Wow - started small and grew larger than initially thought 🤣

It is a big improvement and we should get this merged soon as it touches so many places.

[RobertMueller2]

IMHO config reading error is the only one where we actually would need the extra binary satty-fatal-error-dialog, but IMHO we should not abort here, but just report it and use the defaults. If GTK fails also satty-fatal-error-dialog will fail.

That way we could avoid having an external binary. 🤔

Well, it's the case where a user passes an explicit named config, skipping processing of any defaults. My expectation from a user perspective would be a hard error if the file cannot be read.

Re GTK, that is true. But we also load epoxy in satty main binary, and read the image before the GUI is started.

[robertwidfen]

I am sure I wrote a comment, but cannot find it anymore 🙃 - maybe lost in closed tabs.

IMHO config reading error is the only one where we actually would need the extra binary satty-fatal-error-dialog, but IMHO we should not abort here, but just report it and use the defaults. If GTK fails also satty-fatal-error-dialog will fail.
That way we could avoid having an external binary. 🤔

Well, it's the case where a user passes an explicit named config, skipping processing of any defaults. My expectation from a user perspective would be a hard error if the file cannot be read.

I still think it should not fail hard if config is not found unless started from the terminal. Let's assume I have hyprland binding where for some purpose I start satty with a different config and pip the screenshot to it and at some time I cleanup configs, forgot about this and later in a online meeting take a screenshot - then it will be lost without being able to rewind.

Re GTK, that is true. But we also load epoxy in satty main binary, and read the image before the GUI is started.

I am not deeply into this epoxy stuff - but if the helper app can start and we can catch the error why can we not make the dialog within satty.

I clauded something here but do not know how to test it 🙃
https://github.com/robertwidfen/Satty/tree/fix/show-fatal-errors-within-satty

[RobertMueller2]

Re GTK, that is true. But we also load epoxy in satty main binary, and read the image before the GUI is started.

I am not deeply into this epoxy stuff - but if the helper app can start and we can catch the error why can we not make the dialog within satty.

I clauded something here but do not know how to test it 🙃 https://github.com/robertwidfen/Satty/tree/fix/show-fatal-errors-within-satty

I was not saying we cannot. We either have to start a separate main loop within logging. That was actually in my code yesterday, and that's what your claude attempt does, but that's messy so I decided to toss it out. However, we can do it as part of error handling within main.rs. That's something I'd like to try. And I actually wrote it somewhere above that I was going to.

[RobertMueller2]

I clauded something here but do not know how to test it 🙃 https://github.com/robertwidfen/Satty/tree/fix/show-fatal-errors-within-satty

I think the easiest way to test would be to change the expected library name in epoxy.rs temporarily.

[RobertMueller2]

• removed helper binary in favour of separate main loop creation for error dialog, and reverted changes to workflows, flatpax, flake.nix
• changed --config error to warn
• refactored main() for clearer error handling
• downgraded missing css error to debug

Adding 1 or 2 missing things above: --profile-startup can be refactored.

Should we place the early fatal dialog behind a feature so people who don't want it can remove it from their build individually?

[RobertMueller2]

additional todo: fill out changelog

[robertwidfen]

Will take a closer look on Wednesday ... ;-)

[robertwidfen]

lgtm