Refactored and commented code to be my expressive

Mikayla Maki created

Change summary

crates/terminal/src/terminal.rs         |  45 ++++++++---
crates/terminal/src/terminal_element.rs | 101 ++++++++++++++++----------
2 files changed, 94 insertions(+), 52 deletions(-)

Detailed changes

crates/terminal/src/terminal.rs 🔗

@@ -25,7 +25,6 @@ use workspace::{Item, Workspace};
 use crate::terminal_element::{get_color_at_index, TerminalEl};
 
 //ASCII Control characters on a keyboard
-//Consts -> Structs -> Impls -> Functions, Vaguely in order of importance
 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;
@@ -39,9 +38,11 @@ const DEFAULT_TITLE: &str = "Terminal";
 
 pub mod terminal_element;
 
+///Action for carrying the input to the PTY
 #[derive(Clone, Default, Debug, PartialEq, Eq)]
 pub struct Input(pub String);
 
+///Event to transmit the scroll from the element to the view
 #[derive(Clone, Debug, PartialEq)]
 pub struct ScrollTerminal(pub i32);
 
@@ -51,6 +52,7 @@ actions!(
 );
 impl_internal_actions!(terminal, [Input, ScrollTerminal]);
 
+///Initialize and register all of our action handlers
 pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(Terminal::deploy);
     cx.add_action(Terminal::write_to_pty);
@@ -68,6 +70,7 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(Terminal::scroll_terminal);
 }
 
+///A translation struct for Alacritty to communicate with us from their event loop
 #[derive(Clone)]
 pub struct ZedListener(UnboundedSender<AlacTermEvent>);
 
@@ -77,7 +80,7 @@ impl EventListener for ZedListener {
     }
 }
 
-///A terminal renderer.
+///A terminal view, maintains the PTY's file handles and communicates with the terminal
 pub struct Terminal {
     pty_tx: Notifier,
     term: Arc<FairMutex<Term<ZedListener>>>,
@@ -87,6 +90,7 @@ pub struct Terminal {
     cur_size: SizeInfo,
 }
 
+///Upward flowing events, for changing the title and such
 pub enum Event {
     TitleChanged,
     CloseTerminal,
@@ -128,7 +132,8 @@ impl Terminal {
             ..Default::default()
         };
 
-        //The details here don't matter, the terminal will be resized on layout
+        //The details here don't matter, the terminal will be resized on the first layout
+        //Set to something small for easier debugging
         let size_info = SizeInfo::new(200., 100.0, 5., 5., 0., 0., false);
 
         //Set up the terminal...
@@ -169,7 +174,6 @@ impl Terminal {
         match event {
             AlacTermEvent::Wakeup => {
                 if !cx.is_self_focused() {
-                    //Need to figure out how to trigger a redraw when not in focus
                     self.has_new_content = true; //Change tab content
                     cx.emit(Event::TitleChanged);
                 } else {
@@ -207,6 +211,7 @@ impl Terminal {
                     let term_style = &cx.global::<Settings>().theme.terminal;
                     match index {
                         0..=255 => to_alac_rgb(get_color_at_index(&(index as u8), term_style)),
+                        //These additional values are required to match the Alacritty Colors object's behavior
                         256 => to_alac_rgb(term_style.foreground),
                         257 => to_alac_rgb(term_style.background),
                         258 => to_alac_rgb(term_style.cursor),
@@ -226,8 +231,7 @@ impl Terminal {
                 self.write_to_pty(&Input(format(color)), cx)
             }
             AlacTermEvent::CursorBlinkingChange => {
-                //So, it's our job to set a timer and cause the cursor to blink here
-                //Which means that I'm going to put this off until someone @ Zed looks at it
+                //TODO: Set a timer to blink the cursor on and off
             }
             AlacTermEvent::Bell => {
                 self.has_bell = true;
@@ -237,6 +241,7 @@ impl Terminal {
         }
     }
 
+    ///Resize the terminal and the PTY. This locks the 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();
@@ -245,18 +250,20 @@ impl Terminal {
         }
     }
 
+    ///Scroll the terminal. This locks the terminal
     fn scroll_terminal(&mut self, scroll: &ScrollTerminal, _: &mut ViewContext<Self>) {
         self.term.lock().scroll_display(Scroll::Delta(scroll.0));
     }
 
-    ///Create a new Terminal
+    ///Create a new Terminal in the current working directory or the user's home directory
     fn deploy(workspace: &mut Workspace, _: &Deploy, cx: &mut ViewContext<Workspace>) {
         let project = workspace.project().read(cx);
         let abs_path = project
             .active_entry()
             .and_then(|entry_id| project.worktree_for_entry(entry_id, cx))
             .and_then(|worktree_handle| worktree_handle.read(cx).as_local())
-            .map(|wt| wt.abs_path().to_path_buf());
+            .map(|wt| wt.abs_path().to_path_buf())
+            .or_else(|| Some("~".into()));
 
         workspace.add_item(Box::new(cx.add_view(|cx| Terminal::new(cx, abs_path))), cx);
     }
@@ -266,16 +273,19 @@ impl Terminal {
         self.pty_tx.0.send(Msg::Shutdown).ok();
     }
 
+    ///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 paste(&mut self, _: &Paste, cx: &mut ViewContext<Self>) {
         if let Some(item) = cx.read_from_clipboard() {
             self.write_to_pty(&Input(item.text().to_owned()), cx);
         }
     }
 
+    ///Write the Input payload to the tty. This locks the terminal so we can scroll it.
     fn write_to_pty(&mut self, input: &Input, cx: &mut ViewContext<Self>) {
         //iTerm bell behavior, bell stays until terminal is interacted with
         self.has_bell = false;
@@ -284,38 +294,47 @@ impl Terminal {
         self.pty_tx.notify(input.0.clone().into_bytes());
     }
 
+    ///Send the `up` key
     fn up(&mut self, _: &Up, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(UP_SEQ.to_string()), cx);
     }
 
+    ///Send the `down` key
     fn down(&mut self, _: &Down, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(DOWN_SEQ.to_string()), cx);
     }
 
+    ///Send the `tab` key
     fn tab(&mut self, _: &Tab, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(TAB_CHAR.to_string()), cx);
     }
 
+    ///Send `SIGINT` (`ctrl-c`)
     fn send_sigint(&mut self, _: &Sigint, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(ETX_CHAR.to_string()), cx);
     }
 
+    ///Send the `escape` key
     fn escape(&mut self, _: &Escape, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(ESC_CHAR.to_string()), cx);
     }
 
+    ///Send the `delete` key. TODO: Difference between this and backspace?
     fn del(&mut self, _: &Del, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(DEL_CHAR.to_string()), cx);
     }
 
+    ///Send a carriage return. TODO: May need to check the terminal mode.
     fn carriage_return(&mut self, _: &Return, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(CARRIAGE_RETURN_CHAR.to_string()), cx);
     }
 
+    //Send the `left` key
     fn left(&mut self, _: &Left, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(LEFT_SEQ.to_string()), cx);
     }
 
+    //Send the `right` key
     fn right(&mut self, _: &Right, cx: &mut ViewContext<Self>) {
         self.write_to_pty(&Input(RIGHT_SEQ.to_string()), cx);
     }
@@ -333,10 +352,7 @@ impl View for Terminal {
     }
 
     fn render(&mut self, cx: &mut gpui::RenderContext<'_, Self>) -> ElementBox {
-        TerminalEl::new(cx.handle())
-            .contained()
-            // .with_style(theme.terminal.container)
-            .boxed()
+        TerminalEl::new(cx.handle()).contained().boxed()
     }
 
     fn on_focus(&mut self, cx: &mut ViewContext<Self>) {
@@ -354,7 +370,7 @@ impl Item for Terminal {
 
         if self.has_bell {
             flex.add_child(
-                Svg::new("icons/zap.svg")
+                Svg::new("icons/zap.svg") //TODO: Swap out for a better icon, or at least resize this
                     .with_color(tab_theme.label.text.color)
                     .constrained()
                     .with_width(search_theme.tab_icon_width)
@@ -437,6 +453,7 @@ impl Item for Terminal {
     }
 }
 
+//Convenience method for less lines
 fn to_alac_rgb(color: Color) -> AlacRgb {
     AlacRgb {
         r: color.r,
@@ -451,6 +468,8 @@ mod tests {
     use crate::terminal_element::build_chunks;
     use gpui::TestAppContext;
 
+    ///Basic integration test, can we get the terminal to show up, execute a command,
+    //and produce noticable output?
     #[gpui::test]
     async fn test_terminal(cx: &mut TestAppContext) {
         let terminal = cx.add_view(Default::default(), |cx| Terminal::new(cx, None));

crates/terminal/src/terminal_element.rs 🔗

@@ -42,20 +42,21 @@ pub struct TerminalEl {
 ///Represents a span of cells in a single line in the terminal's grid.
 ///This is used for drawing background rectangles
 #[derive(Debug, Default, Copy, Clone, Eq, PartialEq, PartialOrd, Ord)]
-pub struct LineSpan {
+pub struct RectSpan {
     start: i32,
     end: i32,
     line: usize,
     color: Color,
 }
 
-impl LineSpan {
+///A background color span
+impl RectSpan {
     ///Creates a new LineSpan. `start` must be <= `end`.
     ///If `start` == `end`, then this span is considered to be over a
     /// single cell
-    fn new(start: i32, end: i32, line: usize, color: Color) -> LineSpan {
+    fn new(start: i32, end: i32, line: usize, color: Color) -> RectSpan {
         debug_assert!(start <= end);
-        LineSpan {
+        RectSpan {
             start,
             end,
             line,
@@ -64,6 +65,7 @@ impl LineSpan {
     }
 }
 
+///Helper types so I don't mix these two up
 struct CellWidth(f32);
 struct LineHeight(f32);
 
@@ -117,11 +119,7 @@ impl Element for TerminalEl {
 
         //And we're off! Begin layouting
         let (chunks, line_count) = build_chunks(content.display_iter, &terminal_theme);
-        let backgrounds = chunks
-            .iter()
-            .filter(|(_, _, line_span)| line_span != &LineSpan::default())
-            .map(|(_, _, line_span)| *line_span)
-            .collect();
+
         let shaped_lines = layout_highlighted_chunks(
             chunks
                 .iter()
@@ -133,6 +131,11 @@ impl Element for TerminalEl {
             line_count,
         );
 
+        let backgrounds = chunks
+            .iter()
+            .filter(|(_, _, line_span)| line_span != &RectSpan::default())
+            .map(|(_, _, line_span)| *line_span)
+            .collect();
         let background_rects = make_background_rects(backgrounds, &shaped_lines, &line_height);
 
         let cursor = make_cursor_rect(
@@ -165,8 +168,10 @@ impl Element for TerminalEl {
         layout: &mut Self::LayoutState,
         cx: &mut gpui::PaintContext,
     ) -> Self::PaintState {
+        //Setup element stuff
         cx.scene.push_layer(Some(visible_bounds));
 
+        //Elements are ephemeral, only at paint time do we know what could be clicked by a mouse
         cx.scene.push_mouse_region(MouseRegion {
             view_id: self.view.id(),
             mouse_down: Some(Rc::new(|_, cx| cx.focus_parent_view())),
@@ -236,26 +241,22 @@ impl Element for TerminalEl {
         match event {
             Event::ScrollWheel {
                 delta, position, ..
-            } => {
-                if visible_bounds.contains_point(*position) {
+            } => visible_bounds
+                .contains_point(*position)
+                .then(|| {
                     let vertical_scroll =
                         (delta.y() / layout.line_height.0) * ALACRITTY_SCROLL_MULTIPLIER;
                     cx.dispatch_action(ScrollTerminal(vertical_scroll.round() as i32));
-                    true
-                } else {
-                    false
-                }
-            }
+                })
+                .is_some(),
             Event::KeyDown {
                 input: Some(input), ..
-            } => {
-                if cx.is_parent_view_focused() {
+            } => cx
+                .is_parent_view_focused()
+                .then(|| {
                     cx.dispatch_action(Input(input.to_string()));
-                    true
-                } else {
-                    false
-                }
-            }
+                })
+                .is_some(),
             _ => false,
         }
     }
@@ -303,11 +304,15 @@ fn make_new_size(
     )
 }
 
+///In a single pass, this function generates the background and foreground color info for every item in the grid.
 pub(crate) fn build_chunks(
     grid_iterator: GridIterator<Cell>,
     theme: &TerminalStyle,
-) -> (Vec<(String, Option<HighlightStyle>, LineSpan)>, usize) {
+) -> (Vec<(String, Option<HighlightStyle>, RectSpan)>, usize) {
     let mut line_count: usize = 0;
+    //Every `group_by()` -> `into_iter()` pair needs to be seperated by a local variable so
+    //rust knows where to put everything.
+    //Start by grouping by lines
     let lines = grid_iterator.group_by(|i| i.point.line.0);
     let result = lines
         .into_iter()
@@ -315,10 +320,12 @@ pub(crate) fn build_chunks(
             line_count += 1;
             let mut col_index = 0;
 
+            //Then group by style
             let chunks = line.group_by(|i| cell_style(&i, theme));
             chunks
                 .into_iter()
                 .map(|(style, fragment)| {
+                    //And assemble the styled fragment into it's background and foreground information
                     let str_fragment = fragment.map(|indexed| indexed.c).collect::<String>();
                     let start = col_index;
                     let end = start + str_fragment.len() as i32;
@@ -326,25 +333,30 @@ pub(crate) fn build_chunks(
                     (
                         str_fragment,
                         Some(style.0),
-                        LineSpan::new(start, end, line_count - 1, style.1), //Line count -> Line index
+                        RectSpan::new(start, end, line_count - 1, style.1), //Line count -> Line index
                     )
                 })
+                //Add a \n to the end, as we're using text layouting rather than grid layouts
                 .chain(iter::once(("\n".to_string(), None, Default::default())))
-                .collect::<Vec<(String, Option<HighlightStyle>, LineSpan)>>()
+                .collect::<Vec<(String, Option<HighlightStyle>, RectSpan)>>()
         })
+        //We have a Vec<Vec<>> (Vec of lines of styled chunks), flatten to just Vec<> (the styled chunks)
         .flatten()
-        .collect::<Vec<(String, Option<HighlightStyle>, LineSpan)>>();
+        .collect::<Vec<(String, Option<HighlightStyle>, RectSpan)>>();
     (result, line_count)
 }
 
+///Convert a RectSpan in terms of character offsets, into RectFs of exact offsets
 fn make_background_rects(
-    backgrounds: Vec<LineSpan>,
+    backgrounds: Vec<RectSpan>,
     shaped_lines: &Vec<Line>,
     line_height: &LineHeight,
 ) -> Vec<(RectF, Color)> {
     backgrounds
         .into_iter()
         .map(|line_span| {
+            //This should always be safe, as the shaped lines and backgrounds where derived
+            //At the same time earlier
             let line = shaped_lines
                 .get(line_span.line)
                 .expect("Background line_num did not correspond to a line number");
@@ -361,6 +373,7 @@ fn make_background_rects(
         .collect::<Vec<(RectF, Color)>>()
 }
 
+///Create the rectangle for a cursor, exactly positioned according to the text
 fn make_cursor_rect(
     cursor_point: Point,
     shaped_lines: &Vec<Line>,
@@ -378,10 +391,11 @@ fn make_cursor_rect(
     })
 }
 
+///Convert the Alacritty cell styles to GPUI text styles and background color
 fn cell_style(indexed: &Indexed<&Cell>, style: &TerminalStyle) -> (HighlightStyle, Color) {
     let flags = indexed.cell.flags;
-    let fg = Some(alac_color_to_gpui_color(&indexed.cell.fg, style));
-    let bg = alac_color_to_gpui_color(&indexed.cell.bg, style);
+    let fg = Some(convert_color(&indexed.cell.fg, style));
+    let bg = convert_color(&indexed.cell.bg, style);
 
     let underline = flags.contains(Flags::UNDERLINE).then(|| Underline {
         color: fg,
@@ -399,8 +413,10 @@ fn cell_style(indexed: &Indexed<&Cell>, style: &TerminalStyle) -> (HighlightStyl
     )
 }
 
-fn alac_color_to_gpui_color(alac_color: &AnsiColor, style: &TerminalStyle) -> Color {
+///Converts a 2, 8, or 24 bit color ANSI color to the GPUI equivalent
+fn convert_color(alac_color: &AnsiColor, style: &TerminalStyle) -> Color {
     match alac_color {
+        //Named and theme defined colors
         alacritty_terminal::ansi::Color::Named(n) => match n {
             alacritty_terminal::ansi::NamedColor::Black => style.black,
             alacritty_terminal::ansi::NamedColor::Red => style.red,
@@ -431,14 +447,18 @@ fn alac_color_to_gpui_color(alac_color: &AnsiColor, style: &TerminalStyle) -> Co
             alacritty_terminal::ansi::NamedColor::DimWhite => style.dim_white,
             alacritty_terminal::ansi::NamedColor::BrightForeground => style.bright_foreground,
             alacritty_terminal::ansi::NamedColor::DimForeground => style.dim_foreground,
-        }, //Theme defined
+        },
+        //'True' colors
         alacritty_terminal::ansi::Color::Spec(rgb) => Color::new(rgb.r, rgb.g, rgb.b, u8::MAX),
-        alacritty_terminal::ansi::Color::Indexed(i) => get_color_at_index(i, style), //Color cube weirdness
+        //8 bit, indexed colors
+        alacritty_terminal::ansi::Color::Indexed(i) => get_color_at_index(i, style),
     }
 }
 
+///Converts an 8 bit ANSI color to it's GPUI equivalent.
 pub fn get_color_at_index(index: &u8, style: &TerminalStyle) -> Color {
     match index {
+        //0-15 are the same as the named colors above
         0 => style.black,
         1 => style.red,
         2 => style.green,
@@ -455,16 +475,17 @@ pub fn get_color_at_index(index: &u8, style: &TerminalStyle) -> Color {
         13 => style.bright_magenta,
         14 => style.bright_cyan,
         15 => style.bright_white,
+        //16-231 are mapped to their RGB colors on a 0-5 range per channel
         16..=231 => {
-            let (r, g, b) = rgb_for_index(index); //Split the index into it's rgb components
-            let step = (u8::MAX as f32 / 5.).floor() as u8; //Split the 256 channel range into 5 chunks
-            Color::new(r * step, g * step, b * step, u8::MAX) //Map the [0, 5] rgb components to the [0, 256] channel range
+            let (r, g, b) = rgb_for_index(index); //Split the index into it's ANSI-RGB components
+            let step = (u8::MAX as f32 / 5.).floor() as u8; //Split the RGB range into 5 chunks, with floor so no overflow
+            Color::new(r * step, g * step, b * step, u8::MAX) //Map the ANSI-RGB components to an RGB color
         }
-        //Grayscale from black to white, 0 to 24
+        //232-255 are a 24 step grayscale from black to white
         232..=255 => {
             let i = index - 232; //Align index to 0..24
-            let step = (u8::MAX as f32 / 24.).floor() as u8; //Split the [0,256] range grayscale values into 24 chunks
-            Color::new(i * step, i * step, i * step, u8::MAX) //Map the rgb components to the grayscale range
+            let step = (u8::MAX as f32 / 24.).floor() as u8; //Split the RGB grayscale values into 24 chunks
+            Color::new(i * step, i * step, i * step, u8::MAX) //Map the ANSI-grayscale components to the RGB-grayscale
         }
     }
 }
@@ -486,6 +507,8 @@ fn rgb_for_index(i: &u8) -> (u8, u8, u8) {
     (r, g, b)
 }
 
+///Draws the grid as Alacritty sees it. Useful for checking if there is an inconsistency between
+///Display and conceptual grid.
 #[cfg(debug_assertions)]
 fn draw_debug_grid(bounds: RectF, layout: &mut LayoutState, cx: &mut PaintContext) {
     let width = layout.cur_size.width();