Skip to content
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

Add support for using Ctrl/Alt/Meta with non-character keys #121

Closed
wants to merge 1 commit into from

Conversation

dubrowgn
Copy link

Currently, key modifiers can only be used in conjunction with keys which map to a printable character. This pull request is a refactor that allows key modifiers to be used with any key, such as arrow keys, tab, etc.

@gwenn
Copy link
Collaborator

gwenn commented Jul 16, 2017

I agree that ctrl-left/up/right/down, shift-left/up/right/down are missing.
But I am not sure that your PR is the right approach.
Because it is possible to do ctrl-alt-shift-left.
Maybe we should do something similar to Haskeline ?

@dubrowgn
Copy link
Author

It's a good point. What do you think of something like this then?

pub enum Key {
    Backspace,
    Char(char),
    Delete,
    Down,
    End,
    Enter,
    Esc,
    Home,
    Insert,
    Left,
    Null,
    PageDown,
    PageUp,
    Right,
    Tab,
    Unknown, // not sure about this, from UnknownEscSeq
    Up,
}

pub struct KeyPress {
    pub key: Key,
    pub ctrl: bool,
    pub alt: bool,
    pub shift: bool,
    pub meta: bool,
}

@gwenn
Copy link
Collaborator

gwenn commented Jul 19, 2017

altand meta are the same by default on most unix keyboard, no ?
https://unix.stackexchange.com/questions/28993/what-is-bashs-meta-key:

By default, on most setups, the Meta key is the key labeled Alt. This is because historically, many unix workstations had a key labeled Meta where PCs have a key labeled Alt.

@dubrowgn
Copy link
Author

In many other contexts the meta key is the same as the super key (aka windows, command, etc).

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/metaKey
https://forum.kde.org/viewtopic.php?f=111&t=88599

For clarity, I suppose it would be better to call it super here, instead of meta.

@dubrowgn dubrowgn force-pushed the master branch 2 times, most recently from 1381f82 to 38b9fa1 Compare July 21, 2017 03:57
@dubrowgn
Copy link
Author

So, I'm playing around with this a bit. I ended up implementing the above proposal and several macro helpers. What do you think?

@gwenn
Copy link
Collaborator

gwenn commented Jul 21, 2017

I don't know if the pending PRs will be integrated one day and, if they are, in which order.
But your PR and #103 will generate a lot of conflicts.

@dubrowgn
Copy link
Author

Indeed, although I suspect this PR will cause many conflicts with just about any other change. There is a lot being touched for this.

@gwenn
Copy link
Collaborator

gwenn commented Dec 17, 2018

Would you mind closing the PR ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants