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.

Sign up for GitHub

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

Enable progressive keyboard enhancement protocol #4939

Merged
merged 3 commits into from
Feb 28, 2023
+32 −15
Merged

Enable progressive keyboard enhancement protocol #4939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1abf41
Update crossterm to 0.26.1
the-mikedavis Jan 11, 2023
07e5120
Enable the enhanced keyboard protocol if supported
the-mikedavis Nov 26, 2022
1b50b60
Add shift-backspace keybind alias for backspace (#4937)
lesleyrs Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions book/src/keymap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ experience.
| `Alt-d`, `Alt-Delete` | Delete next word | `delete_word_forward` |
| `Ctrl-u` | Delete to start of line | `kill_to_line_start` |
| `Ctrl-k` | Delete to end of line | `kill_to_line_end` |
| `Ctrl-h`, `Backspace` | Delete previous char | `delete_char_backward` |
| `Ctrl-h`, `Backspace`, `Shift-Backspace` | Delete previous char | `delete_char_backward` |
| `Ctrl-d`, `Delete` | Delete next char | `delete_char_forward` |
| `Ctrl-j`, `Enter` | Insert new line | `insert_newline` |

Expand Down Expand Up @@ -431,7 +431,7 @@ Keys to use within prompt, Remapping currently not supported.
| `Alt-d`, `Alt-Delete`, `Ctrl-Delete` | Delete next word |
| `Ctrl-u` | Delete to start of line |
| `Ctrl-k` | Delete to end of line |
| `Backspace`, `Ctrl-h` | Delete previous char |
| `Backspace`, `Ctrl-h`, `Shift-Backspace` | Delete previous char |
| `Delete`, `Ctrl-d` | Delete next char |
| `Ctrl-s` | Insert a word under doc cursor, may be changed to Ctrl-r Ctrl-w later |
| `Ctrl-p`, `Up` | Select previous history |
Expand Down
2 changes: 1 addition & 1 deletion helix-term/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ which = "4.4"

tokio = { version = "1", features = ["rt", "rt-multi-thread", "io-util", "io-std", "time", "process", "macros", "fs", "parking_lot"] }
tui = { path = "../helix-tui", package = "helix-tui", default-features = false, features = ["crossterm"] }
crossterm = { version = "0.25", features = ["event-stream"] }
crossterm = { version = "0.26", features = ["event-stream"] }
signal-hook = "0.3"
tokio-stream = "0.1"
futures-util = { version = "0.3", features = ["std", "async-await"], default-features = false }
Expand Down
19 changes: 18 additions & 1 deletion helix-term/src/application.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ use anyhow::{Context, Error};
use crossterm::{
event::{
DisableBracketedPaste, DisableFocusChange, DisableMouseCapture, EnableBracketedPaste,
EnableFocusChange, EnableMouseCapture, Event as CrosstermEvent,
EnableFocusChange, EnableMouseCapture, Event as CrosstermEvent, KeyboardEnhancementFlags,
PopKeyboardEnhancementFlags, PushKeyboardEnhancementFlags,
},
execute, terminal,
tty::IsTty,
Expand Down Expand Up @@ -111,6 +112,9 @@ fn restore_term() -> Result<(), Error> {
let mut stdout = stdout();
// reset cursor shape
write!(stdout, "\x1B[0 q")?;
if matches!(terminal::supports_keyboard_enhancement(), Ok(true)) {
execute!(stdout, PopKeyboardEnhancementFlags)?;
}
// Ignore errors on disabling, this might trigger on windows if we call
// disable without calling enable previously
let _ = execute!(stdout, DisableMouseCapture);
Expand Down Expand Up @@ -1056,6 +1060,19 @@ impl Application {
if self.config.load().editor.mouse {
execute!(stdout, EnableMouseCapture)?;
}
if matches!(terminal::supports_keyboard_enhancement(), Ok(true)) {
log::debug!("The enhanced keyboard protocol is supported on this terminal");
execute!(
stdout,
PushKeyboardEnhancementFlags(
KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES
| KeyboardEnhancementFlags::REPORT_ALTERNATE_KEYS
)
)?;
} else {
log::debug!("The enhanced keyboard protocol is not supported on this terminal");
}

Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/keymap/default.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ pub fn default() -> HashMap<Mode, Keymap> {
"A-d" | "A-del" => delete_word_forward,
"C-u" => kill_to_line_start,
"C-k" => kill_to_line_end,
"C-h" | "backspace" => delete_char_backward,
"C-h" | "backspace" | "S-backspace" => delete_char_backward,
Copy link
Member

@pascalkuthe pascalkuthe Feb 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

are there other keybindings that might have worked without this feature but don't work anymore now?
If that is the case should we offer a config option to dsiable this feature? Also I think this technically qualifies as a breaking change because people could have custom keybdings for backspace (or other ambiguous key-codes) like this which would stop working. I added the appropriate label so we don't forget about that

Copy link
Member Author

Choose a reason for hiding this comment

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

Ambiguous keycodes like using C-i for tab or using C-backspace for C-h would change but I think those would be pretty rare. I'm not sure how I feel about a config option for this: if the protocol is enabled then you get more predictable and correct key events - you would only disable it if you wanted to use ambiguous keycodes which I don't think has any advantage

Copy link
Member

Choose a reason for hiding this comment

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

sounds reasonable 👍 I think we should keep the breaking change label tough so this PR is a bit easier to find for people who do experience regressions.

"C-d" | "del" => delete_char_forward,
"C-j" | "ret" => insert_newline,
"tab" => insert_tab,
Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/ui/prompt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ impl Component for Prompt {
alt!('d') | alt!(Delete) | ctrl!(Delete) => self.delete_word_forwards(cx.editor),
ctrl!('k') => self.kill_to_end_of_line(cx.editor),
ctrl!('u') => self.kill_to_start_of_line(cx.editor),
ctrl!('h') | key!(Backspace) => {
ctrl!('h') | key!(Backspace) | shift!(Backspace) => {
self.delete_char_backwards(cx.editor);
(self.callback_fn)(cx, &self.line, PromptEvent::Update);
}
Expand Down
2 changes: 1 addition & 1 deletion helix-tui/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ default = ["crossterm"]
bitflags = "1.3"
cassowary = "0.3"
unicode-segmentation = "1.10"
crossterm = { version = "0.25", optional = true }
crossterm = { version = "0.26", optional = true }
termini = "0.1"
serde = { version = "1", "optional" = true, features = ["derive"]}
helix-view = { version = "0.6", path = "../helix-view", features = ["term"] }
Expand Down
10 changes: 5 additions & 5 deletions helix-tui/src/backend/crossterm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{backend::Backend, buffer::Cell};
use crossterm::{
cursor::{CursorShape, Hide, MoveTo, SetCursorShape, Show},
cursor::{Hide, MoveTo, SetCursorStyle, Show},
execute, queue,
style::{
Attribute as CAttribute, Color as CColor, Print, SetAttribute, SetBackgroundColor,
Expand Down Expand Up @@ -156,12 +156,12 @@ where

fn show_cursor(&mut self, kind: CursorKind) -> io::Result<()> {
let shape = match kind {
CursorKind::Block => CursorShape::Block,
CursorKind::Bar => CursorShape::Line,
CursorKind::Underline => CursorShape::UnderScore,
CursorKind::Block => SetCursorStyle::SteadyBlock,
CursorKind::Bar => SetCursorStyle::SteadyBar,
CursorKind::Underline => SetCursorStyle::SteadyUnderScore,
CursorKind::Hidden => unreachable!(),
};
map_error(execute!(self.buffer, Show, SetCursorShape(shape)))
map_error(execute!(self.buffer, Show, shape))
}

fn get_cursor(&mut self) -> io::Result<(u16, u16)> {
Expand Down
2 changes: 1 addition & 1 deletion helix-view/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ helix-core = { version = "0.6", path = "../helix-core" }
helix-loader = { version = "0.6", path = "../helix-loader" }
helix-lsp = { version = "0.6", path = "../helix-lsp" }
helix-dap = { version = "0.6", path = "../helix-dap" }
crossterm = { version = "0.25", optional = true }
crossterm = { version = "0.26", optional = true }
helix-vcs = { version = "0.6", path = "../helix-vcs" }

# Conversion traits
Expand Down