Make release event an opt-in feature

[TimonPost]

• Makes Event::Release and Event::Repeat an opt in rather then default enabled feature on windows and kitty. The motivation is that it isn't fully cross-platform and causes developers confusion when receiving two events unexpectedly.
• Fixes the issue where people got an Enter release event at application start up.

Fixes: #752

[TimonPost]

@lesleyrs you can potentially keep the pull requests open. Guess it might be cleaner anyways to have this explicit check for press events even if not enabling the feature introduced in this pr.

[lesleyrs]

@lesleyrs you can potentially keep the pull requests open. Guess it might be cleaner anyways to have this explicit check for press events even if not enabling the feature introduced in this pr.

Ok sounds good, I made it a mission to fix those libraries prior to this PR as the breakage was really quite bad for such a simple fix. But I'll try to get them merged still then.

[aschey]

Is there still interest in getting this merged? Seems that a lot of people are still getting bit by this (most recently #973). I'm happy to resolve the merge conflicts and create an updated PR if it would help.

[joshka]

I think this disabling release / repeat events by default is a good idea to continue moving forward on, but making it a feature flag is probably not the right way to have this work. This is because feature unification means that any other dependency your app relies on could enable the feature and break your app's code. This sort of spooky action at a distance is almost as bad as the problem that it's solving.

Instead, what about making this runtime configurable? I'm not sure what exactly that would look like / where the methods to enable this should in general live. This of course also has some downsides (all the arguments about global mutable state apply here), so choosing this over the feature flag approach may not be obvious (I'm only 60/40 on the idea as being a better approach). A third option that avoids global mutable state would be to always ignore the events in event::read() and add an extra method which doesn't ignore them. That feels a bit gross too, so it's likely there's some other approaches that could work here too. I think it's worth exploring that before settling on a specific one.

We also recently added a bunch of helper methods in #949 which explicitly push users towards just getting the key press events rather than all events. Having those methods in your face when using code completion / API docs might make app bugs when using the events less frequent.

[aschey]

I agree, using a feature flag for this is maybe not the best way.

I think we could add a configuration object for this and pass it into a new read_with method or create a new struct.

// Would not report key up or key repeat by default
read()?;

let options = ReadOptions::new().key_up(true).key_repeat(true);
read_with(&options)?;

or

let reader = EventReader::new().key_up(true).key_repeat(true);
reader.read()?;

This would also need to be added to EventStream.

let stream = EventStream::new().key_up(true).key_repeat(true);
stream.next().await?;

There is already an internal event filter so this shouldn't be a large change.