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 CSIu parser for unix #595

Merged
merged 1 commit into from
Feb 6, 2022

Conversation

blaggacao
Copy link
Contributor

@blaggacao blaggacao commented Aug 22, 2021

closes #595

This is used for helix-editor keymaps.

@blaggacao blaggacao marked this pull request as draft August 22, 2021 21:45
@blaggacao blaggacao force-pushed the add-csi-u-parser branch 3 times, most recently from 0d34b04 to 236d94b Compare August 23, 2021 02:14
@blaggacao blaggacao marked this pull request as ready for review August 23, 2021 02:53
Comment on lines +269 to +273
// Issue #371: \n = 0xA, which is also the keycode for Ctrl+J. The only reason we get
// newlines as input is because the terminal converts \r into \n for us. When we
// enter raw mode, we disable that, so \n no longer has any meaning - it's better to
// use Ctrl+J. Waiting to handle it here means it gets picked up later
'\n' if !crate::terminal::sys::is_raw_mode_enabled() => KeyCode::Enter.into(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not fully understand if the raw-mode check is warranted:

CSIu decimal ASCII encoding should be rather unambigous

10 -> LF
13 -> CR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pr have the context: #373

Copy link
Member

@TimonPost TimonPost Oct 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the new line and CTRL+J overlapped in byte code, we need to pick one of them. Since newline doesn't occur when raw mode is enabled we can exclude \n in that case.

Copy link
Contributor Author

@blaggacao blaggacao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One remaining question..

Copy link
Member

@TimonPost TimonPost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late response. Anyhow, thanks! I would prefer to see some tests for this like the other codes. I can assume it work but I don't have the machine to test it myself. Anyhow can't do much harm better some support then non at all.

@TimonPost TimonPost merged commit 979245d into crossterm-rs:master Feb 6, 2022
@blaggacao blaggacao deleted the add-csi-u-parser branch February 6, 2022 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants