-
Notifications
You must be signed in to change notification settings - Fork 568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support astral plane characters on Windows #174
Support astral plane characters on Windows #174
Conversation
This makes it possible to support astral plane characters like emoji. Explained in the comments added.
key_event.key_code.is_printable() was very, very wrong. If you want such a check, do it on the text, not on the key code, because the key code is very limited. I want to be able to enter characters like λ and 🙂 with my Compose key, for example, but the key code will be garbage for such keys. (I see KeyCode::Unknown(RawKeyCode::Windows(231)) for these.) The empty text case improvement is because you can end up with text being empty in some situations and you shouldn’t reset the flashing. For now, I’m just removing the is_printable() check. If anyone feels strongly about preventing non-printable characters from getting into the strings, they should do something cleverer involving Unicode tables.
cool this looks good, will test when I'm next in front of my proper machine. Thanks! |
@@ -431,12 +431,13 @@ impl Widget<String> for TextBoxRaw { | |||
self.reset_cursor_blink(ctx); | |||
} | |||
// Actual typing | |||
k_e if k_e.key_code.is_printable() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with removing this is that control characters get inserted into the string. I agree with you that we shouldn't be looking at the keycode (that's not really a source of truth), but should have an API indicating whether the keyboard event has printable text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what situations do control characters come through this path? I haven’t tried much, but nothing immediately sprang to mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It happens when you press, say, control-A, but the command isn't interpreted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The astral plane stuff looks good, but the is_printable
logic needs more work.
@chris-morgan do you have any interest in revisiting this? |
Not personally at present. In a month or six maybe if no one has by then, but not now. This was just a quick proof of concept, someone else can feel free to take it over and do it however they like. |
okay, thanks. Going to close for now, we can revisit down the road. |
No description provided.