Implement apple standard keybinding events

[nicoburns]

Allows users to respond to standard system-generated keybinding commands on macOS (https://developer.apple.com/documentation/appkit/nsstandardkeybindingresponding). This is important to make text inputs in Rust GUIs feel native on macOS.

The actual implementation of the commands is left up to the user. This implementation merely passes the events into the event loop to allow the user to do so.

• Tested on all platforms changed
• Added an entry to the changelog module if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

[kchibisov]

Such things should be routed via macOS specific application handler and not be a part of WindowEvent.

In general, given that macOS application delegate will be exposed, such very specific things could be done out of tree?

But I'll leave it up to @madsmtm , from my point as long as it stays inside the macOS backend only it's fine by me.

[DJMcNab]

I would like to say that from a Linebender perspective, this is very important.

To clarify, is the suggested way forward to add a method to ApplicationHandler of the form:

fn macos_text_input(&mut self, event_loop: &ActiveEventLoop, window_id: WindowId, event: AppleStandardKeyBindingAction) {}

I think this does belong in the winit tree. Any user who is seriously developing an application using Winit will likely need text input; and to implement text input on macOS properly, you need to have access to these events.

[kchibisov]

I'm not saying that it shouldn't be in winit, I'm saying that it should be on a macOS specific application handler, that's about it.

[nicoburns]

I'm not saying that it shouldn't be in winit, I'm saying that it should be on a macOS specific application handler, that's about it.

When you say "a macOS specific application handler", would a macos_text_input method on the ApplicationHandler trait as suggested above meet that criteria? Or a separate MacOsApplicationHandler trait? Or...?

A MacOsApplicationHandler trait (and presumably similar for other platforms as needed) might actually be quite nice now that I think about it, as it would allow consumers of Winit to feature flag most of the platform-specific code in one place (the impl block).

[kchibisov]

would a macos_text_input method on the ApplicationHandler trait as suggested above meet that criteria

Handler as in a separate trait with macOS events, etc. similar for link openings, etc. User will just implement that macOS trait and you'll call to them based on whether they implement it.

Though, there's an issue with the fact that we accept a generic ApplicationHandler and not dyn one, but we could temporary do vtable based registrations.

In general, see rust-windowing/winit-next#4 for ideas, it's already kind of possible to do.

[nicoburns]

Though, there's an issue with the fact that we accept a generic ApplicationHandler and not dyn one, but we could temporary do vtable based registrations.

Hmm... that dyn approach looks complicated. Would you expect that to land anytime soon?

I'd be keen to avoid blocking practical improvements on designing the perfect API if possible. We can always improve / move the API later, and events/methods that exist in the API but are simply never generated on some platforms seems like a non-issue for users of winit (and is already the case for many winit APIs).

---

Regarding platform-specific traits:

Might a simpler approach to just require the appropriate platform-specific traits (in the run_app method) when winit is compiled for the relevant platform (or feature flag in case of wayland/x11)? Noop default implementations could be provided for each of the methods, so all that someone compiling for e.g. macOS would need to do in addition to their main ApplicationHandler impl is:

#[cfg(target_os = "macos")]
impl MacOsApplicationHandler for MyAppStruct {}

(and similar for other platforms the user is targeting)

One concern I have around platform-specific traits in general is that they might make it more difficult to share functionality between platforms. For example, where does functionality shared between ios/macos or x11/wayland live in this model? Do we end up with a whole hierarchy of traits? Maybe that's fine.

[kchibisov]

I'd be keen to avoid blocking practical improvements on designing the perfect API if possible. We can always improve / move the API later, and events/methods that exist in the API but are simply never generated on some platforms seems like a non-issue for users of winit (and is already the case for many winit APIs).

I'm saying that you already kind of can do so. I'll look myself in the next few days with a ways to adapt this PR to what winit-next does.

[kchibisov]

One concern I have around platform-specific traits in general is that they might make it more
difficult to share functionality between platforms. For example, where does functionality shared
between ios/macos or x11/wayland live in this model? Do we end up with a whole hierarchy of traits?
Maybe that's fine.

Trait usually should be per specific feature and not just platform, see e.g. StartupNotify we have right now. Similar for modifier supplement, etc.

[madsmtm]

@nicoburns: I have updated this PR so that updating the standard key bindings is possible without breaking changes (avoid Other(...), use #[non_exhaustive]). If there is some use-case for Other that I don't know about, then I'd probably prefer not trying to create an enum, and instead just pass the &str directly to the user. I also removed noop:, but unsure if that was correct?

@kchibisov: Additionally, I have made it use an extension trait ApplicationHandlerExtMacOS. I could not get the app.as_any_mut().downcast_mut() trick to work that you have outlined in rust-windowing/winit-next#4, since dyn ApplicationHandler is not Sized, so I've gone with a suboptimal solution for now. Feel free to push to this PR if there's a better way to solve it.

[kchibisov]

Should be fine for now.

The other approach doesn't work with raw downcast you need to have vtable like structure which will require a lot of refactorings in macOS backend, I guess.

[nicoburns]

@madsmtm

I have updated this PR so that updating the standard key bindings is possible without breaking changes (avoid Other(...), use #[non_exhaustive]). If there is some use-case for Other that I don't know about, then I'd probably prefer not trying to create an enum, and instead just pass the &str directly to the user. I also removed noop:, but unsure if that was correct?

The Other case is:

1. Because I am not entirely sure I have covered every action.
2. New macOS versions are liable to add new actions. Without the Other variant we would need to release a new Winit version for those to be usable.

(relatedly, I'm not sure if there are ever noops. Glazier had them, but that may have been for internal use)

I'm actually thinking that just passing through the &str (or even Box<str> or String?) might be a good design:

• It keeps winit's API surface area (+ lines of code) for this feature small
• It is just as efficient, as we're allocating and then parsing a String anyway.
• It would allow the enum in this PR to live in an external crate which could evolve and release much quicker than winit due to it's much smaller scope (I would be happy to create such a crate).
• Crates wanting to implement the actions could depend only on this other crate (and not winit), or could simply accept &str and avoid non-stdlib types entirely.

[madsmtm]

Yeah, the problem with the Other case is that it would make it impossible for us to release a patch version of Winit where we started parsing something, because the users code that used Other would then break.

Anyhow, I've pushed a commit to just pass through &str, I'm much more happy with that myself anyhow.

I've also allowed noop: again, so now this really is just forwarding doCommandBySelector:.

[waywardmonkeys]

Might make sense to just have a module with a bunch of const things for the current set of &str values to help avoid someone making a typo, they can just use the const values instead?

[madsmtm]

Might make sense to just have a module with a bunch of const things for the current set of &str values to help avoid someone making a typo, they can just use the const values instead?

Kinda feel like there isn't much value in that if Nico plans to make a separate crate anyhow. Besides, a typo would be quickly found while testing (and a typo could also be there in our const values).

[nicoburns]

Is there anything blocking this now? (It's looking pretty good to me!)

[nicoburns]

I've pushed a commit which fixes the build. But I'm unable to verify that this is actually (still) working due to #3958

[nicoburns]

Ok, by applying #3959 on top of this PR (which fixes keyboard events on macos in the window example in general), I have been able to verify that this is in fact still working after merging in the latest master.

@kchibisov I believe this is just waiting on final signoff from you, having been already reviewed and reworked by @madsmtm. I know there are a number of Rust GUI frameworks eagerly awaiting this functionality (including mine), so let me know if there's anything I can do to help push it along.

[kchibisov]

This is outdated and should mention the new application trait/etc.