kovidgoyal / kitty

Cross-platform, fast, feature-rich, GPU based terminal
https://sw.kovidgoyal.net/kitty/
GNU General Public License v3.0
24.2k stars 972 forks source link

OSC-99 desktop notifications: icons #7657

Closed dnkl closed 2 months ago

dnkl commented 2 months ago

As mentioned in https://sw.kovidgoyal.net/kitty/desktop-notifications/, icons are not yet supported:

It is possible to extend this escape code to allow specifying an icon for the notification, however, given that some platforms, such as legacy versions of macOS, don’t allow displaying custom images on a notification, it was decided to leave it out of the spec for the time being.

I'm implementing OSC-99 in foot (PR), and I'm also including icon support. Since you seem open to adding it to the spec, here I am :)

For foot, I decided to make it dead simple. The parameter is I (upper case i). We only support symbolic icon names. No filenames, paths (absolute or relative), and no inline pixel data.

I don't see foot ever supporting paths/filenames, but would be open to the spec allowing it.

Inline pixel data is perhaps more interesting. But honestly, given that this is a notification protocol, and not an image protocol, I thought it overkill. But, if there is a chance you want it in the spec, now or in the future, it would be good to consider how it would be encoded. I'm thinking base64 encoded, with another parameter that signals whether the icon is a symbolic name, or pixel data (i.e. somewhat similar to e + p).

What are your thoughts?

Example:

printf '\e]99;I=firefox;Notification with a firefox icon\e\\`
kovidgoyal commented 2 months ago

The problem is that symbolic icon names are useless on non-Linux platforms. As such they aren't really suitable for the protocol. Cant have icon notifications that work only if the terminal is on Linux. You pretty much have to allow transmitting image data as that's the only robust way (over SSH for instance). And then there is the issue that icons cannot be changed in notifications on macOS, at all, Apple doesnt allow it. The best you can do is add the icon as an "attachment" to the notification.

The way it would work is the application would transmit base64 encoded image data in PNG format associated with an string id. The terminal emulator would cache that data. Then for future notifications in that session the application could just transmit the string id. Here session means until the application quits.

You could optionally support the application transmitting a symbolic name, but the protocol would have to make it very clear that this is unlikely to be robust.

Since icons are relatively small and need to be transmitted only once per session I dont think we need file based transmission.

I suggest the g (graphic) key to specify the icon id and n (name) key for the optional symbolic name.

The terminal emulator must prefer the icon based on the symbolic name if both symbolic name and icon data are provided, so that the user is presented with an icon congruent with their icon theme. If the symbolic name cannot be resolved, the icon data must be used.

The spec recommends icon sizes of at least 256x256 pixels.

dnkl commented 2 months ago

Ah, thanks for clarifying the situation, especially wrt Apple.

Given all the above, wouldn't it make sense to make it very clear, in the specification, that icons, regardless of type (inline or symbolic), is a best-effort? And that nothing's guaranteed? That there are/could be both platform and terminal emulator limitations to what is supported. And recommend applications provide both image data, and a symbolic name, for maximum compatibility.

Since icons are relatively small and need to be transmitted only once per session I dont think we need file based transmission.

Agreed

I suggest the g (graphic) key to specify the icon id and n (name) key for the optional symbolic name.

Sounds good. Just so that we understand each other; n would be a standalone option? Or do you expect the terminal to cache that as well (paired with the ID)?

Put another way, should an application do:

printf "\e]99;n=firefox;first notification\e\\"
printf "\e]99;n=firefox;second notification\e\\"

or

printf "\e]99;g=ffox:n=firefox;first notification\e\\"
printf "\e]99;g=ffox;second notification\e\\"

Here session means until the application quits.

"Application" meaning the application running inside the terminal? Or the terminal emulator itself? Should the terminal emulator track the currently running application and automatically reset the icon cache, or should the application somehow notify the terminal emulator?

kovidgoyal commented 2 months ago

On Mon, Jul 22, 2024 at 11:29:20PM -0700, Daniel Eklöf wrote:

Ah, thanks for clarifying the situation, especially wrt Apple.

Given all the above, wouldn't it make sense to make it very clear, in the specification, that icons, regardless of type (inline or symbolic), is a best-effort? And that nothing's guaranteed? That there are/could be both platform and terminal emulator limitations to what is supported. And recommend applications provide both image data, and a symbolic name, for maximum compatibility.

Well icon data should work robustly everywhere, except on macOS it will look like a regular picture on the notification next to the terminal emulators own icon.

Sounds good. Just so that we understand each other; n would be a standalone option? Or do you expect the terminal to cache that as well (paired with the ID)?

It should be cached as well. Application should set n and g once, transmit data using p=icon and all future notifications (in that session) should use g only, no n and no p=icon.

Put another way, should an application do:

printf "\e]99;n=firefox;first notification\e\\"
printf "\e]99;n=firefox;second notification\e\\"

or

printf "\e]99;g=ffox:n=firefox;first notification\e\\"
printf "\e]99;g=ffox;second notification\e\\"

Here session means until the application quits.

"Application" meaning the application running inside the terminal? Or the terminal emulator itself? Should the terminal emulator track the currently running application and automatically reset the icon cache, or should the application somehow notify the terminal emulator?

"Application" means the client running inside the terminal emulator. And no, the terminal emulator should not (and in general cannot) track application state. Icons are small, the terminal emulator can easily maintain a LRU cache of say 16 icons per window. Which should be enough for almost all applications. Indeed, on macOS, to display the icon at all, it needs to exist as a file on the filesystem making a cache natural.

I was going to suggest well behaved applications send g and some sentinel value in the payload to indicate to the terminal that it can clear the icon from its cache. But this would be strictly optional, since caching a few icons isn't a big burden.

dnkl commented 2 months ago

It should be cached as well. Application should set n and g once, transmit data using p=icon and all future notifications (in that session) should use g only, no n and no p=icon.

Ah, you meant to use the payload for icon data. That wasn't obvious to me in your initial description. But yes, that makes sense.

"Application" means the client running inside the terminal emulator. And no, the terminal emulator should not (and in general cannot) track application state

That's what I was hinting at :)

I was going to suggest well behaved applications send g and some sentinel value in the payload to indicate to the terminal that it can clear the icon from its cache. But this would be strictly optional, since caching a few icons isn't a big burden.

That would be ok with me.

One issue I see with the design of the icon cache is nested applications.

The problem is in a way more generic than that; how can an application know the icon ID it's using is still in the cache? It can mostly be solved by spec:ing the cache to be at least N entries. But it still doesn't solve the issue with nested applications.

kovidgoyal commented 2 months ago

The idea is applications use a UUID like identifier for g. And there is no clear full cache operation. To clear an icon from the cache an application sends a g value and the terminal clears only that icon. We can of course theoretically have infinitely nested applications and thus the cache could get full and start evicting things, but in practice with a reasonable cache size I doubt this will actually be an issue. The vast majority of application will use ~5 notification icons. The vast majority of users will never next more than 5 levels deep. So an icon cache of ~32 entries per window should be fine.

And lets remember the failure mode isnt that catastrophic, the worst that happens is the notification gets displayed without the icon.

dnkl commented 2 months ago

That sounds good enough for me.

To clear an icon from the cache an application sends a g value and the terminal clears only that icon.

Yeah, sorry, I misread your previous post. Somehow I read it as "send a sentinel g value", making me think it would clear the entire cache.

I also agree it may not be necessary to implement at all, given how small the cache is.

dnkl commented 2 months ago

Since we pretty much require the icon payload to be base64 encoded, I would suggest that p=icon implies e=1, and that it is spelled out in the specification.

kovidgoyal commented 2 months ago

That would preclude using another encoding in the future. Say base85 for more efficiency.

dnkl commented 2 months ago

Alright, initial implementation is now in https://codeberg.org/dnkl/foot/pulls/1776

Foot gracefully handles an empty icon payload, and can also load the icon cache with a symbolic-only icon at the same time as displaying a notification. I.e. everything below is allowed:

# Load cache, three alternatives
printf '\e]99;g=123:p=icon:e=1;<base64-PNG>\e\\'
printf '\e]99;g=123:n=firefox:p=icon:e=1;<base64-PNG>\e\\'
printf '\e]99;g=123:n=firefox:p=icon;\e\\'

# Display notification using previously loaded icon
printf '\e]99;g=123;this is a notification\e\\'

or, with symbolic name only:

printf '\e]99;g=123:n=firefox;this is a notification\e\\'

If you're strongly against allowing the last variant, I can remove it. It sort of came about automatically, due to how I handle chunking in OSC-99. I still require the g parameter. I.e. it is not possible to do:

printf '\e]99;n=firefix:this is a notification\e\\'
kovidgoyal commented 2 months ago

IMO, the g parameter should not be required. If you are sending a notification with just n and no p=icon then no caching needs to be done, or if it does the terminal emulator can generate its own internal value for g. g is required only so that you can in the future refer to a previously cached icon.

Also, I am thinking of having the spec mandate some well known names that terminals must support for n such as error, warning, info, bug.

dnkl commented 2 months ago

IMO, the g parameter should not be required

Great, I'm definitely on board with that.

Also, I am thinking of having the spec mandate some well known names that terminals must support for n such as error, warning, info, bug.

Might be good idea, for a basic level of platform independence.

kovidgoyal commented 2 months ago

Feel free to comment if you have concerns, this is not set in stone. And note that I have added many other features to the spec, all of which can be exercised using the new notify kitten:

kitten notify --help

dnkl commented 2 months ago

The section "specification of all keys used in the protocol" is not listing "icon" as a valid value for "p". I assume this is just an oversight?

The query response doesn't report availability of the "f" and "t" parameters. Is this intentional? The spec does require implementations to ignore unrecognized parameters, so it should be safe for applications to use them unconditionally without causing any unwanted side effects.

Am I right in assuming that ID=0 is not special cased in any way when replacing/updating a notification? That is, a "bad behaving" application that doesn't set "i" will replace/update a single notification?

kovidgoyal commented 2 months ago

On Wed, Jul 31, 2024 at 12:25:53AM -0700, Daniel Eklöf wrote:

The section "specification of all keys used in the protocol" is not listing "icon" as a valid value for "p". I assume this is just an oversight?

yes.

The query response doesn't report availability of the "f" and "t" parameters. Is this intentional? The spec does require implementations to ignore unrecognized parameters, so it should be safe for applications to use them unconditionally without causing any unwanted side effects.

That's because f and t have no actual functionality tied to them, they are just about giving users a nicer notification filtering experience. And yes since unrecognized keys must be ignored, applications can send them unconditionally.

Am I right in assuming that ID=0 is not special cased in any way when replacing/updating a notification? That is, a "bad behaving" application that doesn't set "i" will replace/update a single notification?

Without i, an update/replace is the same as creating a new notification. I shall add a line to the spec to make this clear.