macOS: Only check is_repeat to send ReceivedCharacter. Fixes #1488

[jayache80]

Checking state.is_key_down is redundant, since we are handling keyDown.
Moreover, state.is_key_down can be affected by a previous repeated key's
keyUp.

• Tested on all platforms changed
• Compilation warnings were addressed
• cargo fmt has been run on this branch
• cargo doc builds successfully
• Added an entry to CHANGELOG.md 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

[jayache80]

#1488

[kjmph]

Oh super interesting! This is fixed as well with the proposed pull request #1449. Maybe they should be merged?

[jayache80]

@kjmph Yes- that pull request is frying bigger fish and rips out the “pass_along” kludge altogether. I’ll try to get to a macOS system to build #1449 to verify. If so, then this pull request should be closed

[saysjonathan]

#1449 builds for me when merged into current master 38fcceb.

[jayache80]

Yes, I also merged master 38fcceb into #1449 and built alacritty with it. It solves the alacritty key repeat issue alacritty/alacritty#2788 that this pull request (#1489) was created to solve, but #1449 does it in a better way. #1449 would also resolve issue #1488.

This pull request (#1489) should be closed in lieu of #1449.

[jayache80]

Well, it's been a while, Alacritty still has the key repetition bug that would be fixed by this pull-request. This pull-request would've been superseded by #1449 but alas that has yet to be ironed out. I think this should be reconsidered.

[ArturKovacs]

Hi @jayache80! You are right, this PR should be merged as the other one is stuck currently. However there are two things:

1. Considering that state.is_key_down was only used for this purpse, please remove it's declaration from ViewState and all other places it shows up.
2. Given that it's not at all obvious why it is okay to remove this condition, please add the following explanation to the commit message:

`is_key_down` was only set to true when `insertText` was called. Therefore
`is_keydown` actually meant to store whether the most recently pressed key
generated an `insertText` event, at which, winit produces a `ReceivedCharacter`.
The issue is that `insertText` is *not* called for “key repeat”, but winit wants
to send `ReceivedCharacter` events for “key repeat” too. To solve this, the
`is_key_down` variable was then checked in the `key_down` function to determine
whether it was valid to produce repeated `ReceivedCharacter` events during “key
repeat”. However this check is not needed since `ReceivedCharacter` must always
be called if the key event has a non-empty `characters` property.

Furthermore `is_key_down` didn’t actually store whether the previously pressed
character had an `insertText` event, because it was incorrectly set to false on
every “key up”. This meant that if two keys were pressed consecutively then the
first was released, then `is_key_down` was set to false even if the most recent
keypress did actually produce an `insertText`.

[jayache80]

@ArturKovacs I've rebased the branch off latest master and ripped out is_key_down from ViewState completely. I've included your explanation in the commit message.

[ArturKovacs]

Awesome! I forgot one thing last time: please add an entry to CHANGELOG.md along the lines of:

- On macOS, fixed not emitting `ReceivedCharacter` during some key repeat events.

[jayache80]

@ArturKovacs

please add an entry to CHANGELOG.md

Added an entry to the changelog (amended to the commit).

Is the title of this pull-request still appropriate, or should I edit the title?

[ArturKovacs]

Is the title of this pull-request still appropriate, or should I edit the title?

It is somewhat obscure but I'm not sure how it could be improved, so I'll leave this up to you