-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat(minttea): supporting ctrl-key modifiers #45
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
open Riot | ||
|
||
type modifier = Ctrl | ||
type key_event = { key : string; modifier : modifier option } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question. Another thought is that to avoid the type modifier = | No_modifier | Ctrl Stuffing this type with combinations would also make it easier to check for Ctrl + Shift by just using What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is really good, I just added option as it made sense that no modifiers could just be None, but either way works fine. But I do enjoy not have Some/Nones. To me this suggestion is great |
||
|
||
type key = | ||
| Up | ||
| Down | ||
|
@@ -9,7 +12,7 @@ type key = | |
| Escape | ||
| Backspace | ||
| Enter | ||
| Key of string | ||
| Key of key_event | ||
|
||
let key_to_string key = | ||
match key with | ||
|
@@ -21,7 +24,8 @@ let key_to_string key = | |
| Escape -> "<esc>" | ||
| Backspace -> "<backspace>" | ||
| Enter -> "<enter>" | ||
| Key k -> k | ||
| Key { key; modifier = Some Ctrl } -> "<c-" ^ key ^ ">" | ||
| Key { key; modifier = None } -> key | ||
|
||
type t = | ||
| KeyDown of key | ||
|
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.
suggestion. Could we move the
modifier
up to theKeyDown
constructor?I think this would allow you to pass in modifier for the special keys too, while keeping all the pattern matching ergonomic:
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.
Didn't think of this, I think this is way better and more ergonomic, will add these changes