New key mapping system in place and working

Mikayla Maki created

Change summary

assets/keymaps/default.json              |  16 +--
crates/terminal/src/connection.rs        |   7 
crates/terminal/src/connection/events.rs |  45 ++++++---
crates/terminal/src/terminal.rs          | 116 ++++++-------------------
crates/terminal/src/terminal_element.rs  |  41 +++-----
5 files changed, 84 insertions(+), 141 deletions(-)

Detailed changes

assets/keymaps/default.json 🔗

@@ -407,18 +407,16 @@
     {
         "context": "Terminal",
         "bindings": {
-            "ctrl-c": "terminal::Sigint",
-            "escape": "terminal::Escape",
-            "ctrl-d": "terminal::Quit",
-            "backspace": "terminal::Del",
-            "enter": "terminal::Return",
-            "left": "terminal::Left",
-            "right": "terminal::Right",
+            // Overrides for global bindings, remove at your own risk:
             "up": "terminal::Up",
             "down": "terminal::Down",
-            "tab": "terminal::Tab",
+            "escape": "terminal::Escape",
+            "enter": "terminal::Enter",
+            "ctrl-c": "terminal::CtrlC",
+            // Useful terminal actions:
+            "cmd-c": "terminal::Copy",
             "cmd-v": "terminal::Paste",
-            "cmd-c": "terminal::Copy"
+            "cmd-k": "terminal::Clear"
         }
     },
     {

crates/terminal/src/connection.rs 🔗

@@ -15,7 +15,7 @@ use futures::{channel::mpsc::unbounded, StreamExt};
 use settings::Settings;
 use std::{collections::HashMap, path::PathBuf, sync::Arc};
 
-use gpui::{ClipboardItem, CursorStyle, Entity, KeyDownEvent, ModelContext};
+use gpui::{keymap::Keystroke, ClipboardItem, CursorStyle, Entity, ModelContext};
 
 use crate::{
     color_translation::{get_color_at_index, to_alac_rgb},
@@ -187,10 +187,11 @@ impl TerminalConnection {
         self.term.lock().clear_screen(ClearMode::Saved);
     }
 
-    pub fn try_keystroke(&mut self, key_down: &KeyDownEvent) -> bool {
+    pub fn try_keystroke(&mut self, keystroke: &Keystroke) -> bool {
         let guard = self.term.lock();
         let mode = guard.mode();
-        let esc = to_esc_str(key_down, mode);
+        let esc = to_esc_str(keystroke, mode);
+        dbg!(&esc);
         drop(guard);
         if esc.is_some() {
             self.write_to_pty(esc.unwrap());

crates/terminal/src/connection/events.rs 🔗

@@ -1,15 +1,12 @@
 use alacritty_terminal::term::TermMode;
-use gpui::{keymap::Keystroke, KeyDownEvent};
+use gpui::keymap::Keystroke;
 
 pub enum ModifierCombinations {
     None,
     Alt,
     Ctrl,
     Shift,
-    AltCtrl,
-    AltShift,
     CtrlShift,
-    AltCtrlShift,
     Other,
 }
 
@@ -20,20 +17,24 @@ impl ModifierCombinations {
             (true, false, false, false) => ModifierCombinations::Alt,
             (false, true, false, false) => ModifierCombinations::Ctrl,
             (false, false, true, false) => ModifierCombinations::Shift,
-            (true, true, false, false) => ModifierCombinations::AltCtrl,
-            (true, false, true, false) => ModifierCombinations::AltShift,
             (false, true, true, false) => ModifierCombinations::CtrlShift,
-            (true, true, true, false) => ModifierCombinations::AltCtrlShift,
             _ => ModifierCombinations::Other,
         }
     }
 }
 
-pub fn to_esc_str(event: &KeyDownEvent, mode: &TermMode) -> Option<String> {
-    let modifiers = ModifierCombinations::new(&event.keystroke);
+pub fn to_esc_str(keystroke: &Keystroke, mode: &TermMode) -> Option<String> {
+    let modifiers = ModifierCombinations::new(&keystroke);
 
     // Manual Bindings including modifiers
-    let manual_esc_str = match (event.keystroke.key.as_ref(), modifiers) {
+    let manual_esc_str = match (keystroke.key.as_ref(), modifiers) {
+        //Basic special keys
+        ("space", ModifierCombinations::None) => Some(" ".to_string()),
+        ("tab", ModifierCombinations::None) => Some("\x09".to_string()),
+        ("escape", ModifierCombinations::None) => Some("\x1b".to_string()),
+        ("enter", ModifierCombinations::None) => Some("\x0d".to_string()),
+        ("backspace", ModifierCombinations::None) => Some("\x7f".to_string()),
+        //Interesting escape codes
         ("tab", ModifierCombinations::Shift) => Some("\x1b[Z".to_string()),
         ("backspace", ModifierCombinations::Alt) => Some("\x1b\x7f".to_string()),
         ("backspace", ModifierCombinations::Shift) => Some("\x7f".to_string()),
@@ -111,7 +112,7 @@ pub fn to_esc_str(event: &KeyDownEvent, mode: &TermMode) -> Option<String> {
         ("f19", ModifierCombinations::None) => Some("\x1b[33~".to_string()),
         ("f20", ModifierCombinations::None) => Some("\x1b[34~".to_string()),
         // NumpadEnter, Action::Esc("\n".into());
-        //Make all mappings for caret notation keys!
+        //Mappings for caret notation keys
         ("a", ModifierCombinations::Ctrl) => Some("\x01".to_string()), //1
         ("A", ModifierCombinations::CtrlShift) => Some("\x01".to_string()), //1
         ("b", ModifierCombinations::Ctrl) => Some("\x02".to_string()), //2
@@ -178,8 +179,8 @@ pub fn to_esc_str(event: &KeyDownEvent, mode: &TermMode) -> Option<String> {
     }
 
     // Automated bindings applying modifiers
-    let modifier_code = modifier_code(&event.keystroke);
-    let modified_esc_str = match event.keystroke.key.as_ref() {
+    let modifier_code = modifier_code(&keystroke);
+    let modified_esc_str = match keystroke.key.as_ref() {
         "up" => Some(format!("\x1b[1;{}A", modifier_code)),
         "down" => Some(format!("\x1b[1;{}B", modifier_code)),
         "right" => Some(format!("\x1b[1;{}C", modifier_code)),
@@ -217,10 +218,26 @@ pub fn to_esc_str(event: &KeyDownEvent, mode: &TermMode) -> Option<String> {
     }
 
     // Fallback to keystroke input sent directly
-    return event.input.clone();
+    if keystroke.key.chars().count() == 1 {
+        dbg!("This should catch space", &keystroke.key);
+        return Some(keystroke.key.clone());
+    } else {
+        None
+    }
 }
 
 /*
+New keybindings test plan:
+
+Is the terminal still usable?  YES!
+Do ctrl-shift-[X] and ctrl-[x] do the same thing? I THINK SO
+Does ctrl-l work? YES
+Does tab work? YES
+Do all the global overrides (up, down, enter, escape, ctrl-c) work? => YES
+Space also doesn't work YES!
+
+
+
 So, to match  alacritty keyboard handling, we need to check APP_CURSOR, and ALT_SCREEN
 
 And we need to convert the strings that GPUI returns to keys

crates/terminal/src/terminal.rs 🔗

@@ -5,7 +5,6 @@ pub mod terminal_element;
 
 use alacritty_terminal::{
     event::{Event as AlacTermEvent, EventListener},
-    grid::Scroll,
     term::SizeInfo,
 };
 
@@ -14,7 +13,7 @@ use dirs::home_dir;
 use editor::Input;
 use futures::channel::mpsc::UnboundedSender;
 use gpui::{
-    actions, elements::*, impl_internal_actions, AppContext, ClipboardItem, Entity, ModelHandle,
+    actions, elements::*, keymap::Keystroke, AppContext, ClipboardItem, Entity, ModelHandle,
     MutableAppContext, View, ViewContext,
 };
 use modal::deploy_modal;
@@ -27,16 +26,6 @@ use workspace::{Item, Workspace};
 
 use crate::terminal_element::TerminalEl;
 
-//ASCII Control characters on a keyboard
-const ETX_CHAR: char = 3_u8 as char; //'End of text', the control code for 'ctrl-c'
-const TAB_CHAR: char = 9_u8 as char;
-const CARRIAGE_RETURN_CHAR: char = 13_u8 as char;
-const ESC_CHAR: char = 27_u8 as char; // == \x1b
-const DEL_CHAR: char = 127_u8 as char;
-const LEFT_SEQ: &str = "\x1b[D";
-const RIGHT_SEQ: &str = "\x1b[C";
-const UP_SEQ: &str = "\x1b[A";
-const DOWN_SEQ: &str = "\x1b[B";
 const DEBUG_TERMINAL_WIDTH: f32 = 1000.; //This needs to be wide enough that the prompt can fill the whole space.
 const DEBUG_TERMINAL_HEIGHT: f32 = 200.;
 const DEBUG_CELL_WIDTH: f32 = 5.;
@@ -52,44 +41,34 @@ pub struct ScrollTerminal(pub i32);
 actions!(
     terminal,
     [
-        Sigint,
-        Escape,
-        Del,
-        Return,
-        Left,
-        Right,
+        Deploy,
         Up,
         Down,
-        Tab,
+        CtrlC,
+        Escape,
+        Enter,
         Clear,
         Copy,
         Paste,
-        Deploy,
-        Quit,
         DeployModal
     ]
 );
-impl_internal_actions!(terminal, [ScrollTerminal]);
 
 ///Initialize and register all of our action handlers
 pub fn init(cx: &mut MutableAppContext) {
-    cx.add_action(Terminal::deploy);
-    cx.add_action(Terminal::send_sigint);
-    cx.add_action(Terminal::escape);
-    cx.add_action(Terminal::quit);
-    cx.add_action(Terminal::del);
-    cx.add_action(Terminal::carriage_return);
-    cx.add_action(Terminal::left);
-    cx.add_action(Terminal::right);
+    //Global binding overrrides
+    cx.add_action(Terminal::send_ctrl_c);
     cx.add_action(Terminal::up);
     cx.add_action(Terminal::down);
-    cx.add_action(Terminal::tab);
+    cx.add_action(Terminal::escape);
+    cx.add_action(Terminal::enter);
+    //Useful terminal actions
+    cx.add_action(Terminal::deploy);
+    cx.add_action(deploy_modal);
     cx.add_action(Terminal::copy);
     cx.add_action(Terminal::paste);
-    cx.add_action(Terminal::scroll_terminal);
     cx.add_action(Terminal::input);
     cx.add_action(Terminal::clear);
-    cx.add_action(deploy_modal);
 }
 
 ///A translation struct for Alacritty to communicate with us from their event loop
@@ -168,15 +147,6 @@ impl Terminal {
         }
     }
 
-    ///Scroll the terminal. This locks the terminal
-    fn scroll_terminal(&mut self, scroll: &ScrollTerminal, cx: &mut ViewContext<Self>) {
-        self.connection
-            .read(cx)
-            .term
-            .lock()
-            .scroll_display(Scroll::Delta(scroll.0));
-    }
-
     fn input(&mut self, Input(text): &Input, cx: &mut ViewContext<Self>) {
         self.connection.update(cx, |connection, _| {
             //TODO: This is probably not encoding UTF8 correctly (see alacritty/src/input.rs:L825-837)
@@ -200,11 +170,6 @@ impl Terminal {
         workspace.add_item(Box::new(cx.add_view(|cx| Terminal::new(wd, false, cx))), cx);
     }
 
-    ///Tell Zed to close us
-    fn quit(&mut self, _: &Quit, cx: &mut ViewContext<Self>) {
-        cx.emit(Event::CloseTerminal);
-    }
-
     ///Attempt to paste the clipboard into the terminal
     fn copy(&mut self, _: &Copy, cx: &mut ViewContext<Self>) {
         let term = self.connection.read(cx).term.lock();
@@ -224,66 +189,39 @@ impl Terminal {
         }
     }
 
-    ///Send the `up` key
+    ///Synthesize the keyboard event corresponding to 'up'
     fn up(&mut self, _: &Up, cx: &mut ViewContext<Self>) {
         self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(UP_SEQ.to_string());
+            connection.try_keystroke(&Keystroke::parse("up").unwrap());
         });
     }
 
-    ///Send the `down` key
+    ///Synthesize the keyboard event corresponding to 'down'
     fn down(&mut self, _: &Down, cx: &mut ViewContext<Self>) {
         self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(DOWN_SEQ.to_string());
+            connection.try_keystroke(&Keystroke::parse("down").unwrap());
         });
     }
 
-    ///Send the `tab` key
-    fn tab(&mut self, _: &Tab, cx: &mut ViewContext<Self>) {
+    ///Synthesize the keyboard event corresponding to 'ctrl-c'
+    fn send_ctrl_c(&mut self, _: &CtrlC, cx: &mut ViewContext<Self>) {
         self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(TAB_CHAR.to_string());
+            connection.try_keystroke(&Keystroke::parse("ctrl-c").unwrap());
         });
     }
 
-    ///Send `SIGINT` (`ctrl-c`)
-    fn send_sigint(&mut self, _: &Sigint, cx: &mut ViewContext<Self>) {
-        self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(ETX_CHAR.to_string());
-        });
-    }
-
-    ///Send the `escape` key
+    ///Synthesize the keyboard event corresponding to 'escape'
     fn escape(&mut self, _: &Escape, cx: &mut ViewContext<Self>) {
         self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(ESC_CHAR.to_string());
-        });
-    }
-
-    ///Send the `delete` key. TODO: Difference between this and backspace?
-    fn del(&mut self, _: &Del, cx: &mut ViewContext<Self>) {
-        self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(DEL_CHAR.to_string());
-        });
-    }
-
-    ///Send a carriage return. TODO: May need to check the terminal mode.
-    fn carriage_return(&mut self, _: &Return, cx: &mut ViewContext<Self>) {
-        self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(CARRIAGE_RETURN_CHAR.to_string());
-        });
-    }
-
-    //Send the `left` key
-    fn left(&mut self, _: &Left, cx: &mut ViewContext<Self>) {
-        self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(LEFT_SEQ.to_string());
+            connection.try_keystroke(&Keystroke::parse("escape").unwrap());
         });
     }
 
-    //Send the `right` key
-    fn right(&mut self, _: &Right, cx: &mut ViewContext<Self>) {
+    ///Synthesize the keyboard event corresponding to 'enter'
+    fn enter(&mut self, _: &Enter, cx: &mut ViewContext<Self>) {
+        dbg!("Here!");
         self.connection.update(cx, |connection, _| {
-            connection.write_to_pty(RIGHT_SEQ.to_string());
+            connection.try_keystroke(&Keystroke::parse("enter").unwrap());
         });
     }
 }
@@ -476,7 +414,7 @@ mod tests {
             terminal.connection.update(cx, |connection, _| {
                 connection.write_to_pty("expr 3 + 4".to_string());
             });
-            terminal.carriage_return(&Return, cx);
+            terminal.enter(&Enter, cx);
         });
 
         cx.set_condition_duration(Some(Duration::from_secs(2)));
@@ -501,7 +439,7 @@ mod tests {
             terminal.connection.update(cx, |connection, _| {
                 connection.write_to_pty("expr 3 + 4".to_string());
             });
-            terminal.carriage_return(&Return, cx);
+            terminal.enter(&Enter, cx);
         });
 
         terminal

crates/terminal/src/terminal_element.rs 🔗

@@ -1,5 +1,5 @@
 use alacritty_terminal::{
-    grid::{Dimensions, GridIterator, Indexed},
+    grid::{Dimensions, GridIterator, Indexed, Scroll},
     index::{Column as GridCol, Line as GridLine, Point, Side},
     selection::{Selection, SelectionRange, SelectionType},
     sync::FairMutex,
@@ -31,9 +31,7 @@ use theme::TerminalStyle;
 use std::{cmp::min, ops::Range, rc::Rc, sync::Arc};
 use std::{fmt::Debug, ops::Sub};
 
-use crate::{
-    color_translation::convert_color, connection::TerminalConnection, ScrollTerminal, ZedListener,
-};
+use crate::{color_translation::convert_color, connection::TerminalConnection, ZedListener};
 
 ///Scrolling is unbearably sluggish by default. Alacritty supports a configurable
 ///Scroll multiplier that is set to 3 by default. This will be removed when I
@@ -359,25 +357,6 @@ impl Element for TerminalEl {
         _paint: &mut Self::PaintState,
         cx: &mut gpui::EventContext,
     ) -> bool {
-        //The problem:
-        //Depending on the terminal mode, we either send an escape sequence
-        //OR update our own data structures.
-        //e.g. scrolling. If we do smooth scrolling, then we need to check if
-        //we own scrolling and then if so, do our scrolling thing.
-        //Ok, so the terminal connection should have APIs for querying it semantically
-        //something like `should_handle_scroll()`. This means we need a handle to the connection.
-        //Actually, this is the only time that this app needs to talk to the outer world.
-        //TODO for scrolling rework: need a way of intercepting Home/End/PageUp etc.
-        //Sometimes going to scroll our own internal buffer, sometimes going to send ESC
-        //
-        //Same goes for key events
-        //Actually, we don't use the terminal at all in dispatch_event code, the view
-        //Handles it all. Check how the editor implements scrolling, is it view-level
-        //or element level?
-
-        //Question: Can we continue dispatching to the view, so it can talk to the connection
-        //Or should we instead add a connection into here?
-
         match event {
             Event::ScrollWheel(ScrollWheelEvent {
                 delta, position, ..
@@ -386,10 +365,18 @@ impl Element for TerminalEl {
                 .then(|| {
                     let vertical_scroll =
                         (delta.y() / layout.line_height.0) * ALACRITTY_SCROLL_MULTIPLIER;
-                    cx.dispatch_action(ScrollTerminal(vertical_scroll.round() as i32));
+
+                    if let Some(connection) = self.connection.upgrade(cx.app) {
+                        connection.update(cx.app, |connection, _| {
+                            connection
+                                .term
+                                .lock()
+                                .scroll_display(Scroll::Delta(vertical_scroll.round() as i32));
+                        })
+                    }
                 })
                 .is_some(),
-            Event::KeyDown(e @ KeyDownEvent { .. }) => {
+            Event::KeyDown(KeyDownEvent { keystroke, .. }) => {
                 if !cx.is_parent_view_focused() {
                     return false;
                 }
@@ -397,7 +384,9 @@ impl Element for TerminalEl {
                 self.connection
                     .upgrade(cx.app)
                     .map(|connection| {
-                        connection.update(cx.app, |connection, _| connection.try_keystroke(e))
+                        connection.update(cx.app, |connection, _| {
+                            connection.try_keystroke(dbg!(keystroke))
+                        })
                     })
                     .unwrap_or(false)
             }