Beginning rewrite of affected systems

Mikayla Maki created

Change summary

crates/terminal/src/connection.rs        |  10 ++
crates/terminal/src/connection/events.rs | 112 ++++++++++++++++++++++++++
crates/terminal/src/keyboard_to_esc.rs   |  21 ----
crates/terminal/src/terminal.rs          |  18 +--
crates/terminal/src/terminal_element.rs  |  76 +++++++++++------
5 files changed, 177 insertions(+), 60 deletions(-)

Detailed changes

crates/terminal/src/connection.rs 🔗

@@ -1,3 +1,5 @@
+mod events;
+
 use alacritty_terminal::{
     config::{Config, PtyConfig},
     event::{Event as AlacTermEvent, Notify},
@@ -123,7 +125,7 @@ impl TerminalConnection {
             AlacTermEvent::PtyWrite(out) => self.write_to_pty(out, cx),
             AlacTermEvent::MouseCursorDirty => {
                 //Calculate new cursor style.
-                //TODO
+                //TODO: alacritty/src/input.rs:L922-L939
                 //Check on correctly handling mouse events for terminals
                 cx.platform().set_cursor_style(CursorStyle::Arrow); //???
             }
@@ -173,6 +175,12 @@ impl TerminalConnection {
         self.term.lock().scroll_display(Scroll::Bottom);
         self.pty_tx.notify(input);
     }
+
+    ///Resize the terminal and the PTY. This locks the terminal.
+    pub fn set_size(&mut self, new_size: SizeInfo) {
+        self.pty_tx.0.send(Msg::Resize(new_size)).ok();
+        self.term.lock().resize(new_size);
+    }
 }
 
 impl Drop for TerminalConnection {

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

@@ -0,0 +1,112 @@
+use alacritty_terminal::term::TermMode;
+use gpui::KeyDownEvent;
+
+pub fn _to_esc_str(event: &KeyDownEvent, _mode: TermMode) -> String {
+    let key = event.keystroke.key.clone();
+    let modifiers = (
+        event.keystroke.alt,
+        event.keystroke.cmd,
+        event.keystroke.ctrl,
+        event.keystroke.shift,
+    );
+    match (key.as_str(), modifiers) {
+        //NOTE TO SELF: Terminals can rewrite the color index with OSC, use alacritty colors properly.
+
+        //ctrl-l
+        //shift-tab
+        //alt-back
+        //shift-back
+        //shift + Home, end, page up, page down + NOT alt screen => We handle those
+        //shift + Home, end, page up, page down + alt screen => Send escape sequence
+        ("l", (false, false, true, false)) => "\x0c".to_string(),
+        _ => event.input.clone().unwrap().clone(),
+    }
+}
+
+/*
+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
+
+And we need a way of easily declaring and matching a modifier pattern on those keys
+
+And we need to block writing the input to the pty if any of these match
+
+And I need to figure out how to express this in a cross platform way
+
+And a way of optionally interfacing this with actions for rebinding in defaults.json
+
+Design notes:
+I would like terminal mode checking to be concealed behind the TerminalConnection in as many ways as possible.
+Alacritty has a lot of stuff intermixed for it's input handling. TerminalConnection should be in charge
+of anything that needs to conform to a standard that isn't handled by Term, e.g.:
+- Reporting mouse events correctly.
+- Reporting scrolls -> Depends on MOUSE_MODE, ALT_SCREEN, and ALTERNATE_SCROLL, etc.
+- Correctly bracketing a paste
+- Storing changed colors
+- Focus change sequence
+
+Scrolling might be handled internally or externally, need a way to ask. Everything else should probably happen internally.
+
+Standards/OS compliance is in connection.rs.
+This takes GPUI events and translates them to the correct terminal stuff
+This means that standards compliance outside of connection should be kept to a minimum. Yes, this feels good.
+Connection needs to be split up then, into a bunch of event handlers
+
+NOTE, THE FOLLOWING HAS 2 BINDINGS:
+K, ModifiersState::LOGO, Action::Esc("\x0c".into());
+K, ModifiersState::LOGO, Action::ClearHistory; => ctx.terminal_mut().clear_screen(ClearMode::Saved),
+
+
+Handled in therminal:
+L,    ModifiersState::CTRL, Action::Esc("\x0c".into());
+Tab,  ModifiersState::SHIFT, Action::Esc("\x1b[Z".into());
+Backspace, ModifiersState::ALT, Action::Esc("\x1b\x7f".into());
+Backspace, ModifiersState::SHIFT, Action::Esc("\x7f".into());
+Home,     ModifiersState::SHIFT, +BindingMode::ALT_SCREEN, Action::Esc("\x1b[1;2H".into());
+End,      ModifiersState::SHIFT, +BindingMode::ALT_SCREEN, Action::Esc("\x1b[1;2F".into());
+PageUp,   ModifiersState::SHIFT, +BindingMode::ALT_SCREEN, Action::Esc("\x1b[5;2~".into());
+PageDown, ModifiersState::SHIFT, +BindingMode::ALT_SCREEN, Action::Esc("\x1b[6;2~".into());
+Home,  +BindingMode::APP_CURSOR, Action::Esc("\x1bOH".into());
+Home,  ~BindingMode::APP_CURSOR, Action::Esc("\x1b[H".into());
+End,   +BindingMode::APP_CURSOR, Action::Esc("\x1bOF".into());
+End,   ~BindingMode::APP_CURSOR, Action::Esc("\x1b[F".into());
+Up,    +BindingMode::APP_CURSOR, Action::Esc("\x1bOA".into());
+Up,    ~BindingMode::APP_CURSOR, Action::Esc("\x1b[A".into());
+Down,  +BindingMode::APP_CURSOR, Action::Esc("\x1bOB".into());
+Down,  ~BindingMode::APP_CURSOR, Action::Esc("\x1b[B".into());
+Right, +BindingMode::APP_CURSOR, Action::Esc("\x1bOC".into());
+Right, ~BindingMode::APP_CURSOR, Action::Esc("\x1b[C".into());
+Left,  +BindingMode::APP_CURSOR, Action::Esc("\x1bOD".into());
+Left,  ~BindingMode::APP_CURSOR, Action::Esc("\x1b[D".into());
+Back,        Action::Esc("\x7f".into());
+Insert,      Action::Esc("\x1b[2~".into());
+Delete,      Action::Esc("\x1b[3~".into());
+PageUp,      Action::Esc("\x1b[5~".into());
+PageDown,    Action::Esc("\x1b[6~".into());
+F1,          Action::Esc("\x1bOP".into());
+F2,          Action::Esc("\x1bOQ".into());
+F3,          Action::Esc("\x1bOR".into());
+F4,          Action::Esc("\x1bOS".into());
+F5,          Action::Esc("\x1b[15~".into());
+F6,          Action::Esc("\x1b[17~".into());
+F7,          Action::Esc("\x1b[18~".into());
+F8,          Action::Esc("\x1b[19~".into());
+F9,          Action::Esc("\x1b[20~".into());
+F10,         Action::Esc("\x1b[21~".into());
+F11,         Action::Esc("\x1b[23~".into());
+F12,         Action::Esc("\x1b[24~".into());
+F13,         Action::Esc("\x1b[25~".into());
+F14,         Action::Esc("\x1b[26~".into());
+F15,         Action::Esc("\x1b[28~".into());
+F16,         Action::Esc("\x1b[29~".into());
+F17,         Action::Esc("\x1b[31~".into());
+F18,         Action::Esc("\x1b[32~".into());
+F19,         Action::Esc("\x1b[33~".into());
+F20,         Action::Esc("\x1b[34~".into());
+NumpadEnter, Action::Esc("\n".into());
+
+MAC:
+Insert, ModifiersState::SHIFT,  Action::Esc("\x1b[2;2~".into());
+
+*/

crates/terminal/src/keyboard_to_esc.rs 🔗

@@ -1,21 +0,0 @@
-use gpui::KeyDownEvent;
-
-pub fn to_esc_str(event: &KeyDownEvent) -> String {
-    let key = event.keystroke.key.clone();
-    let modifiers = (
-        event.keystroke.alt,
-        event.keystroke.cmd,
-        event.keystroke.ctrl,
-        event.keystroke.shift,
-    );
-    match (key.as_str(), modifiers) {
-        //ctrl-l
-        //shift-tab
-        //alt-back
-        //shift-back
-        //shift + Home, end, page up, page down + NOT alt screen => We handle those
-        //shift + Home, end, page up, page down + alt screen => Send escape sequence
-        ("l", (false, false, true, false)) => "\x0c".to_string(),
-        _ => event.input.clone().unwrap().clone(),
-    }
-}

crates/terminal/src/terminal.rs 🔗

@@ -1,12 +1,10 @@
 mod color_translation;
 pub mod connection;
-mod keyboard_to_esc;
 mod modal;
 pub mod terminal_element;
 
 use alacritty_terminal::{
     event::{Event as AlacTermEvent, EventListener},
-    event_loop::Msg,
     grid::Scroll,
     term::SizeInfo,
 };
@@ -169,14 +167,6 @@ impl Terminal {
         }
     }
 
-    ///Resize the terminal and the PTY. This locks the terminal.
-    fn set_size(&self, new_size: SizeInfo, cx: &mut MutableAppContext) {
-        self.connection.update(cx, |connection, _| {
-            connection.pty_tx.0.send(Msg::Resize(new_size)).ok();
-            connection.term.lock().resize(new_size);
-        })
-    }
-
     ///Scroll the terminal. This locks the terminal
     fn scroll_terminal(&mut self, scroll: &ScrollTerminal, cx: &mut ViewContext<Self>) {
         self.connection
@@ -188,6 +178,7 @@ impl Terminal {
 
     fn input(&mut self, Input(text): &Input, cx: &mut ViewContext<Self>) {
         self.connection.update(cx, |connection, cx| {
+            //TODO: This is probably not encoding UTF8 correctly (see alacritty/src/input.rs:L825-837)
             connection.write_to_pty(text.clone(), cx);
         });
 
@@ -297,7 +288,12 @@ impl View for Terminal {
     }
 
     fn render(&mut self, cx: &mut gpui::RenderContext<'_, Self>) -> ElementBox {
-        let element = TerminalEl::new(cx.handle()).contained();
+        let element = {
+            let connection_handle = self.connection.clone().downgrade();
+            let view_id = cx.view_id();
+            TerminalEl::new(view_id, connection_handle, self.modal).contained()
+        };
+
         if self.modal {
             let settings = cx.global::<Settings>();
             let container_style = settings.theme.terminal.modal_container;

crates/terminal/src/terminal_element.rs 🔗

@@ -21,7 +21,7 @@ use gpui::{
     json::json,
     text_layout::{Line, RunStyle},
     Event, FontCache, KeyDownEvent, MouseRegion, PaintContext, Quad, ScrollWheelEvent,
-    SizeConstraint, TextLayoutCache, WeakViewHandle,
+    SizeConstraint, TextLayoutCache, WeakModelHandle,
 };
 use itertools::Itertools;
 use ordered_float::OrderedFloat;
@@ -32,8 +32,7 @@ use std::{cmp::min, ops::Range, rc::Rc, sync::Arc};
 use std::{fmt::Debug, ops::Sub};
 
 use crate::{
-    color_translation::convert_color, keyboard_to_esc::to_esc_str, ScrollTerminal, Terminal,
-    ZedListener,
+    color_translation::convert_color, connection::TerminalConnection, ScrollTerminal, ZedListener,
 };
 
 ///Scrolling is unbearably sluggish by default. Alacritty supports a configurable
@@ -47,8 +46,11 @@ const ALACRITTY_SCROLL_MULTIPLIER: f32 = 3.;
 const DEBUG_GRID: bool = false;
 
 ///The GPUI element that paints the terminal.
+///We need to keep a reference to the view for mouse events, do we need it for any other terminal stuff, or can we move that to connection?
 pub struct TerminalEl {
-    view: WeakViewHandle<Terminal>,
+    connection: WeakModelHandle<TerminalConnection>,
+    view_id: usize,
+    modal: bool,
 }
 
 ///New type pattern so I don't mix these two up
@@ -98,8 +100,16 @@ pub struct LayoutState {
 }
 
 impl TerminalEl {
-    pub fn new(view: WeakViewHandle<Terminal>) -> TerminalEl {
-        TerminalEl { view }
+    pub fn new(
+        view_id: usize,
+        connection: WeakModelHandle<TerminalConnection>,
+        modal: bool,
+    ) -> TerminalEl {
+        TerminalEl {
+            view_id,
+            connection,
+            modal,
+        }
     }
 }
 
@@ -121,22 +131,19 @@ impl Element for TerminalEl {
             cx.font_cache()
                 .em_advance(text_style.font_id, text_style.font_size),
         );
-        let view_handle = self.view.upgrade(cx).unwrap();
+        let connection_handle = self.connection.upgrade(cx).unwrap();
 
         //Tell the view our new size. Requires a mutable borrow of cx and the view
         let cur_size = make_new_size(constraint, &cell_width, &line_height);
         //Note that set_size locks and mutates the terminal.
-        view_handle.update(cx.app, |view, cx| view.set_size(cur_size, cx));
-
-        //Now that we're done with the mutable portion, grab the immutable settings and view again
-        let view = view_handle.read(cx);
+        connection_handle.update(cx.app, |connection, _| connection.set_size(cur_size));
 
         let (selection_color, terminal_theme) = {
             let theme = &(cx.global::<Settings>()).theme;
             (theme.editor.selection.selection, &theme.terminal)
         };
 
-        let terminal_mutex = view_handle.read(cx).connection.read(cx).term.clone();
+        let terminal_mutex = connection_handle.read(cx).term.clone();
         let term = terminal_mutex.lock();
         let grid = term.grid();
         let cursor_point = grid.cursor.point;
@@ -149,7 +156,7 @@ impl Element for TerminalEl {
             &text_style,
             terminal_theme,
             cx.text_layout_cache,
-            view.modal,
+            self.modal,
             content.selection,
         );
 
@@ -193,7 +200,7 @@ impl Element for TerminalEl {
         });
         drop(term);
 
-        let background_color = if view.modal {
+        let background_color = if self.modal {
             terminal_theme.colors.modal_background
         } else {
             terminal_theme.colors.background
@@ -232,7 +239,7 @@ impl Element for TerminalEl {
             attach_mouse_handlers(
                 origin,
                 cur_size,
-                self.view.id(),
+                self.view_id,
                 &layout.terminal,
                 visible_bounds,
                 cx,
@@ -352,6 +359,25 @@ 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, ..
@@ -363,18 +389,14 @@ impl Element for TerminalEl {
                     cx.dispatch_action(ScrollTerminal(vertical_scroll.round() as i32));
                 })
                 .is_some(),
-            Event::KeyDown(
-                e @ KeyDownEvent {
-                    input: Some(input), ..
-                },
-            ) => {
-                dbg!(e);
-                cx.is_parent_view_focused()
-                    .then(|| {
-                        cx.dispatch_action(Input(to_esc_str(e)));
-                    })
-                    .is_some()
-            }
+            Event::KeyDown(KeyDownEvent {
+                input: Some(input), ..
+            }) => cx
+                .is_parent_view_focused()
+                .then(|| {
+                    cx.dispatch_action(Input(input.to_string()));
+                })
+                .is_some(),
             Event::KeyDown(e) => {
                 dbg!(e);
                 false