Added a test with max, cludged a fix for resizing

Mikayla Maki created

Change summary

.swp                                    |   0 
assets/keymaps/default.json             |  18 +-
crates/terminal/Cargo.toml              |   3 
crates/terminal/src/terminal.rs         |  97 ++++++++---
crates/terminal/src/terminal_element.rs | 217 +++++++++++++++-----------
styles/src/themes/cave.ts               |   5 
6 files changed, 204 insertions(+), 136 deletions(-)

Detailed changes

assets/keymaps/default.json 🔗

@@ -356,16 +356,16 @@
     {
         "context": "Terminal",
         "bindings": {
-            "ctrl-c": "terminal::SIGINT",
-            "escape": "terminal::ESCAPE",
+            "ctrl-c": "terminal::Sigint",
+            "escape": "terminal::Escape",
             "ctrl-d": "terminal::Quit",
-            "backspace": "terminal::DEL",
-            "enter": "terminal::RETURN",
-            "left": "terminal::LEFT",
-            "right": "terminal::RIGHT",
-            "up": "terminal::UP",
-            "down": "terminal::DOWN",
-            "tab": "terminal::TAB",
+            "backspace": "terminal::Del",
+            "enter": "terminal::Return",
+            "left": "terminal::Left",
+            "right": "terminal::Right",
+            "up": "terminal::Up",
+            "down": "terminal::Down",
+            "tab": "terminal::Tab",
             "cmd-v": "terminal::Paste"
         }
     }

crates/terminal/Cargo.toml 🔗

@@ -20,3 +20,6 @@ smallvec = { version = "1.6", features = ["union"] }
 mio-extras = "2.0.6"
 futures = "0.3"
 ordered-float = "2.1.1"
+
+[dev-dependencies]
+gpui = { path = "../gpui", features = ["test-support"] }

crates/terminal/src/terminal.rs 🔗

@@ -22,7 +22,7 @@ use smallvec::SmallVec;
 use std::sync::Arc;
 use workspace::{Item, Workspace};
 
-use crate::element::TerminalEl;
+use crate::terminal_element::TerminalEl;
 
 //ASCII Control characters on a keyboard
 //Consts -> Structs -> Impls -> Functions, Vaguely in order of importance
@@ -37,16 +37,19 @@ const UP_SEQ: &str = "\x1b[A";
 const DOWN_SEQ: &str = "\x1b[B";
 const DEFAULT_TITLE: &str = "Terminal";
 
-pub mod element;
+pub mod terminal_element;
 
 #[derive(Clone, Default, Debug, PartialEq, Eq)]
 pub struct Input(pub String);
 
+#[derive(Clone, Debug, PartialEq)]
+pub struct ScrollTerminal(pub i32);
+
 actions!(
     terminal,
-    [SIGINT, ESCAPE, DEL, RETURN, LEFT, RIGHT, UP, DOWN, TAB, Clear, Paste, Deploy, Quit]
+    [Sigint, Escape, Del, Return, Left, Right, Up, Down, Tab, Clear, Paste, Deploy, Quit]
 );
-impl_internal_actions!(terminal, [Input]);
+impl_internal_actions!(terminal, [Input, ScrollTerminal]);
 
 pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(Terminal::deploy);
@@ -62,6 +65,7 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(Terminal::down);
     cx.add_action(Terminal::tab);
     cx.add_action(Terminal::paste);
+    cx.add_action(Terminal::scroll_terminal);
 }
 
 #[derive(Clone)]
@@ -74,15 +78,16 @@ impl EventListener for ZedListener {
 }
 
 ///A terminal renderer.
-struct Terminal {
+pub struct Terminal {
     pty_tx: Notifier,
     term: Arc<FairMutex<Term<ZedListener>>>,
     title: String,
     has_new_content: bool,
     has_bell: bool, //Currently using iTerm bell, show bell emoji in tab until input is received
+    cur_size: SizeInfo,
 }
 
-enum Event {
+pub enum Event {
     TitleChanged,
     CloseTerminal,
     Activate,
@@ -124,7 +129,7 @@ impl Terminal {
         };
 
         //TODO figure out how to derive this better
-        let size_info = SizeInfo::new(400., 100.0, 5., 5., 0., 0., false);
+        let size_info = SizeInfo::new(200., 100.0, 5., 5., 0., 0., false);
 
         //Set up the terminal...
         let term = Term::new(&config, size_info, ZedListener(events_tx.clone()));
@@ -151,6 +156,7 @@ impl Terminal {
             pty_tx,
             has_new_content: false,
             has_bell: false,
+            cur_size: size_info,
         }
     }
 
@@ -214,6 +220,18 @@ impl Terminal {
         }
     }
 
+    fn set_size(&mut self, new_size: SizeInfo) {
+        if new_size != self.cur_size {
+            self.pty_tx.0.send(Msg::Resize(new_size)).ok();
+            self.term.lock().resize(new_size);
+            self.cur_size = new_size;
+        }
+    }
+
+    fn scroll_terminal(&mut self, scroll: &ScrollTerminal, _: &mut ViewContext<Self>) {
+        self.term.lock().scroll_display(Scroll::Delta(scroll.0));
+    }
+
     ///Create a new Terminal
     fn deploy(workspace: &mut Workspace, _: &Deploy, cx: &mut ViewContext<Workspace>) {
         workspace.add_item(Box::new(cx.add_view(|cx| Terminal::new(cx))), cx);
@@ -242,39 +260,39 @@ impl Terminal {
         self.pty_tx.notify(input.0.clone().into_bytes());
     }
 
-    fn up(&mut self, _: &UP, cx: &mut ViewContext<Self>) {
+    fn up(&mut self, _: &Up, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(UP_SEQ.to_string()), cx);
     }
 
-    fn down(&mut self, _: &DOWN, cx: &mut ViewContext<Self>) {
+    fn down(&mut self, _: &Down, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(DOWN_SEQ.to_string()), cx);
     }
 
-    fn tab(&mut self, _: &TAB, cx: &mut ViewContext<Self>) {
+    fn tab(&mut self, _: &Tab, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(TAB_CHAR.to_string()), cx);
     }
 
-    fn send_sigint(&mut self, _: &SIGINT, cx: &mut ViewContext<Self>) {
+    fn send_sigint(&mut self, _: &Sigint, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(ETX_CHAR.to_string()), cx);
     }
 
-    fn escape(&mut self, _: &ESCAPE, cx: &mut ViewContext<Self>) {
+    fn escape(&mut self, _: &Escape, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(ESC_CHAR.to_string()), cx);
     }
 
-    fn del(&mut self, _: &DEL, cx: &mut ViewContext<Self>) {
+    fn del(&mut self, _: &Del, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(DEL_CHAR.to_string()), cx);
     }
 
-    fn carriage_return(&mut self, _: &RETURN, cx: &mut ViewContext<Self>) {
+    fn carriage_return(&mut self, _: &Return, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(CARRIAGE_RETURN_CHAR.to_string()), cx);
     }
 
-    fn left(&mut self, _: &LEFT, cx: &mut ViewContext<Self>) {
+    fn left(&mut self, _: &Left, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(LEFT_SEQ.to_string()), cx);
     }
 
-    fn right(&mut self, _: &RIGHT, cx: &mut ViewContext<Self>) {
+    fn right(&mut self, _: &Right, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(RIGHT_SEQ.to_string()), cx);
     }
 }
@@ -291,20 +309,10 @@ impl View for Terminal {
     }
 
     fn render(&mut self, cx: &mut gpui::RenderContext<'_, Self>) -> ElementBox {
-        let _theme = cx.global::<Settings>().theme.clone();
-
-        //TODO: derive this
-        let size_info = SizeInfo::new(400., 100.0, 5., 5., 0., 0., false);
-
-        TerminalEl::new(
-            self.term.clone(),
-            self.pty_tx.0.clone(),
-            size_info,
-            cx.view_id(),
-        )
-        .contained()
-        // .with_style(theme.terminal.container)
-        .boxed()
+        TerminalEl::new(cx.handle())
+            .contained()
+            // .with_style(theme.terminal.container)
+            .boxed()
     }
 
     fn on_focus(&mut self, cx: &mut ViewContext<Self>) {
@@ -404,3 +412,32 @@ impl Item for Terminal {
         matches!(event, &Event::Activate)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::terminal_element::build_chunks;
+    use gpui::TestAppContext;
+
+    #[gpui::test]
+    async fn test_terminal(cx: &mut TestAppContext) {
+        let terminal = cx.add_view(Default::default(), |cx| Terminal::new(cx));
+
+        terminal.update(cx, |terminal, cx| {
+            terminal.write_to_pty(&Input(("expr 3 + 4".to_string()).to_string()), cx);
+            terminal.carriage_return(&Return, cx);
+        });
+
+        terminal
+            .condition(cx, |terminal, _cx| {
+                let term = terminal.term.clone();
+                let (chunks, _) = build_chunks(
+                    term.lock().renderable_content().display_iter,
+                    &Default::default(),
+                );
+                let content = chunks.iter().map(|e| e.0.trim()).collect::<String>();
+                content.contains("7")
+            })
+            .await;
+    }
+}

crates/terminal/src/element.rs → crates/terminal/src/terminal_element.rs 🔗

@@ -1,14 +1,11 @@
 use alacritty_terminal::{
     ansi::Color as AnsiColor,
-    event_loop::Msg,
-    grid::{Indexed, Scroll},
+    grid::{GridIterator, Indexed},
     index::Point,
-    sync::FairMutex,
     term::{
         cell::{Cell, Flags},
         SizeInfo,
     },
-    Term,
 };
 use gpui::{
     color::Color,
@@ -17,38 +14,28 @@ use gpui::{
     geometry::{rect::RectF, vector::vec2f},
     json::json,
     text_layout::Line,
-    Event, MouseRegion, Quad,
+    Event, MouseRegion, PaintContext, Quad, WeakViewHandle,
 };
-use mio_extras::channel::Sender;
 use ordered_float::OrderedFloat;
 use settings::Settings;
-use std::{rc::Rc, sync::Arc};
+use std::rc::Rc;
 use theme::TerminalStyle;
 
-use crate::{Input, ZedListener};
+use crate::{Input, ScrollTerminal, Terminal};
 
 const ALACRITTY_SCROLL_MULTIPLIER: f32 = 3.;
+const MAGIC_VISUAL_WIDTH_MULTIPLIER: f32 = 1.28; //1/8th + .003 so we bias long instead of short
+
+#[cfg(debug_assertions)]
+const DEBUG_GRID: bool = false;
 
 pub struct TerminalEl {
-    term: Arc<FairMutex<Term<ZedListener>>>,
-    pty_tx: Sender<Msg>,
-    size: SizeInfo,
-    view_id: usize,
+    view: WeakViewHandle<Terminal>,
 }
 
 impl TerminalEl {
-    pub fn new(
-        term: Arc<FairMutex<Term<ZedListener>>>,
-        pty_tx: Sender<Msg>,
-        size: SizeInfo,
-        view_id: usize,
-    ) -> TerminalEl {
-        TerminalEl {
-            term,
-            pty_tx,
-            size,
-            view_id,
-        }
+    pub fn new(view: WeakViewHandle<Terminal>) -> TerminalEl {
+        TerminalEl { view }
     }
 }
 
@@ -57,6 +44,7 @@ pub struct LayoutState {
     line_height: f32,
     em_width: f32,
     cursor: Option<RectF>,
+    cur_size: SizeInfo,
 }
 
 impl Element for TerminalEl {
@@ -68,88 +56,56 @@ impl Element for TerminalEl {
         constraint: gpui::SizeConstraint,
         cx: &mut gpui::LayoutContext,
     ) -> (gpui::geometry::vector::Vector2F, Self::LayoutState) {
+        let view = self.view.upgrade(cx).unwrap();
         let size = constraint.max;
         let settings = cx.global::<Settings>();
         let editor_theme = &settings.theme.editor;
-        let terminal_theme = &settings.theme.terminal;
-        //Get terminal
-        let mut term = self.term.lock();
 
         //Set up text rendering
-        let font_cache = cx.font_cache();
-
-        let font_family_id = settings.buffer_font_family;
-        let font_family_name = cx.font_cache().family_name(font_family_id).unwrap();
-        let font_properties = Default::default();
-        let font_id = font_cache
-            .select_font(font_family_id, &font_properties)
-            .unwrap();
-        let font_size = settings.buffer_font_size;
-
         let text_style = TextStyle {
             color: editor_theme.text_color,
             font_family_id: settings.buffer_font_family,
-            font_family_name,
-            font_id,
-            font_size,
+            font_family_name: cx
+                .font_cache()
+                .family_name(settings.buffer_font_family)
+                .unwrap(),
+            font_id: cx
+                .font_cache()
+                .select_font(settings.buffer_font_family, &Default::default())
+                .unwrap(),
+            font_size: settings.buffer_font_size,
             font_properties: Default::default(),
             underline: Default::default(),
         };
 
         let line_height = cx.font_cache.line_height(text_style.font_size);
-        let em_width = cx
+        let cell_width = cx
             .font_cache()
             .em_width(text_style.font_id, text_style.font_size)
-            + 2.;
-
-        //Resize terminal
-        let new_size = SizeInfo::new(size.x(), size.y(), em_width, line_height, 0., 0., false);
-        if !new_size.eq(&self.size) {
-            self.pty_tx.send(Msg::Resize(new_size)).ok();
-            term.resize(new_size);
-            self.size = new_size;
-        }
-
-        //Start rendering
-        let content = term.renderable_content();
-
-        let mut lines: Vec<(String, Option<HighlightStyle>)> = vec![];
-        let mut last_line = 0;
-        let mut line_count = 1;
-        let mut cur_chunk = String::new();
-
-        let mut cur_highlight = HighlightStyle {
-            color: Some(Color::white()),
-            ..Default::default()
-        };
-
-        for cell in content.display_iter {
-            let Indexed {
-                point: Point { line, .. },
-                cell: Cell {
-                    c, fg, flags, .. // TODO: Add bg and flags
-                }, //TODO: Learn what 'CellExtra does'
-            } = cell;
-
-            let new_highlight = make_style_from_cell(fg, flags, &terminal_theme);
+            * MAGIC_VISUAL_WIDTH_MULTIPLIER;
+
+        let new_size = SizeInfo::new(
+            size.x() - cell_width, //Padding. Really should make this more explicit
+            size.y(),
+            cell_width,
+            line_height,
+            0.,
+            0.,
+            false,
+        );
+        view.update(cx.app, |view, _cx| {
+            view.set_size(new_size);
+        });
 
-            if line != last_line {
-                line_count += 1;
-                cur_chunk.push('\n');
-                last_line = line.0;
-            }
+        let settings = cx.global::<Settings>();
+        let terminal_theme = &settings.theme.terminal;
+        let term = view.read(cx).term.lock();
 
-            if new_highlight != cur_highlight {
-                lines.push((cur_chunk.clone(), Some(cur_highlight.clone())));
-                cur_chunk.clear();
-                cur_highlight = new_highlight;
-            }
-            cur_chunk.push(*c)
-        }
-        lines.push((cur_chunk, Some(cur_highlight)));
+        let content = term.renderable_content();
+        let (chunks, line_count) = build_chunks(content.display_iter, &terminal_theme);
 
         let shaped_lines = layout_highlighted_chunks(
-            lines.iter().map(|(text, style)| (text.as_str(), *style)),
+            chunks.iter().map(|(text, style)| (text.as_str(), *style)),
             &text_style,
             cx.text_layout_cache,
             &cx.font_cache,
@@ -167,7 +123,7 @@ impl Element for TerminalEl {
             let cursor_x = layout_line.x_for_index(content.cursor.point.column.0);
             cursor = Some(RectF::new(
                 vec2f(cursor_x, cursor_line as f32 * line_height),
-                vec2f(em_width, line_height),
+                vec2f(cell_width, line_height),
             ));
         }
 
@@ -176,8 +132,9 @@ impl Element for TerminalEl {
             LayoutState {
                 lines: shaped_lines,
                 line_height,
-                em_width,
+                em_width: cell_width,
                 cursor,
+                cur_size: new_size,
             },
         )
     }
@@ -192,7 +149,7 @@ impl Element for TerminalEl {
         cx.scene.push_layer(Some(visible_bounds));
 
         cx.scene.push_mouse_region(MouseRegion {
-            view_id: self.view_id,
+            view_id: self.view.id(),
             discriminant: None,
             bounds: visible_bounds,
             hover: None,
@@ -205,7 +162,7 @@ impl Element for TerminalEl {
             right_mouse_down_out: None,
         });
 
-        let origin = bounds.origin() + vec2f(layout.em_width, 0.);
+        let origin = bounds.origin() + vec2f(layout.em_width, 0.); //Padding
 
         let mut line_origin = origin;
         for line in &layout.lines {
@@ -229,6 +186,11 @@ impl Element for TerminalEl {
             });
         }
 
+        #[cfg(debug_assertions)]
+        if DEBUG_GRID {
+            draw_debug_grid(bounds, layout, cx);
+        }
+
         cx.scene.pop_layer();
     }
 
@@ -248,8 +210,7 @@ impl Element for TerminalEl {
                 if visible_bounds.contains_point(*position) {
                     let vertical_scroll =
                         (delta.y() / layout.line_height) * ALACRITTY_SCROLL_MULTIPLIER;
-                    let scroll = Scroll::Delta(vertical_scroll.round() as i32);
-                    self.term.lock().scroll_display(scroll);
+                    cx.dispatch_action(ScrollTerminal(vertical_scroll.round() as i32));
                     true
                 } else {
                     false
@@ -282,6 +243,47 @@ impl Element for TerminalEl {
     }
 }
 
+pub(crate) fn build_chunks(
+    grid_iterator: GridIterator<Cell>,
+    theme: &TerminalStyle,
+) -> (Vec<(String, Option<HighlightStyle>)>, usize) {
+    let mut lines: Vec<(String, Option<HighlightStyle>)> = vec![];
+    let mut last_line = 0;
+    let mut line_count = 1;
+    let mut cur_chunk = String::new();
+
+    let mut cur_highlight = HighlightStyle {
+        color: Some(Color::white()),
+        ..Default::default()
+    };
+
+    for cell in grid_iterator {
+        let Indexed {
+          point: Point { line, .. },
+          cell: Cell {
+              c, fg, flags, .. // TODO: Add bg and flags
+          }, //TODO: Learn what 'CellExtra does'
+      } = cell;
+
+        let new_highlight = make_style_from_cell(fg, flags, theme);
+
+        if line != last_line {
+            line_count += 1;
+            cur_chunk.push('\n');
+            last_line = line.0;
+        }
+
+        if new_highlight != cur_highlight {
+            lines.push((cur_chunk.clone(), Some(cur_highlight.clone())));
+            cur_chunk.clear();
+            cur_highlight = new_highlight;
+        }
+        cur_chunk.push(*c)
+    }
+    lines.push((cur_chunk, Some(cur_highlight)));
+    (lines, line_count)
+}
+
 fn make_style_from_cell(fg: &AnsiColor, flags: &Flags, style: &TerminalStyle) -> HighlightStyle {
     let fg = Some(alac_color_to_gpui_color(fg, style));
     let underline = if flags.contains(Flags::UNDERLINE) {
@@ -337,3 +339,32 @@ fn alac_color_to_gpui_color(allac_color: &AnsiColor, style: &TerminalStyle) -> C
         alacritty_terminal::ansi::Color::Indexed(_) => Color::white(), //Color cube weirdness
     }
 }
+
+#[cfg(debug_assertions)]
+fn draw_debug_grid(bounds: RectF, layout: &mut LayoutState, cx: &mut PaintContext) {
+    let width = layout.cur_size.width();
+    let height = layout.cur_size.height();
+    //Alacritty uses 'as usize', so shall we.
+    for col in 0..(width / layout.em_width).round() as usize {
+        cx.scene.push_quad(Quad {
+            bounds: RectF::new(
+                bounds.origin() + vec2f((col + 1) as f32 * layout.em_width, 0.),
+                vec2f(1., height),
+            ),
+            background: Some(Color::green()),
+            border: Default::default(),
+            corner_radius: 0.,
+        });
+    }
+    for row in 0..((height / layout.line_height) + 1.0).round() as usize {
+        cx.scene.push_quad(Quad {
+            bounds: RectF::new(
+                bounds.origin() + vec2f(layout.em_width, row as f32 * layout.line_height),
+                vec2f(width, 1.),
+            ),
+            background: Some(Color::green()),
+            border: Default::default(),
+            corner_radius: 0.,
+        });
+    }
+}

styles/src/themes/cave.ts 🔗

@@ -25,7 +25,4 @@ const ramps = {
 };
 
 export const dark = createTheme(`${name}-dark`, false, ramps);
-export const light = createTheme(`${name}-light`, true, ramps);
-
-console.log(JSON.stringify(dark.ramps.neutral.domain()))
-console.log(JSON.stringify(light.ramps.neutral.domain()))
+export const light = createTheme(`${name}-light`, true, ramps);