Fixed cursor blinking, added other cursor shape rendering

Mikayla Maki created

Change summary

assets/settings/default.json           |   8 +-
crates/gpui/src/platform/mac/window.rs |   1 
crates/settings/src/settings.rs        |   4 
crates/terminal/src/connected_el.rs    | 103 ++++++++++++++-------------
crates/terminal/src/connected_view.rs  |  12 +-
crates/terminal/src/terminal.rs        |   7 -
6 files changed, 70 insertions(+), 65 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -105,12 +105,12 @@
         //Set the cursor blinking behavior in the terminal.
         //May take 4 values:
         // 1. Never blink the cursor, ignoring the terminal mode
-        //        "blinking": "never",
-        // 2. Default the cursor blink to on, but allow the terminal to 
-        //    turn blinking off
+        //        "blinking": "off",
+        // 2. Default the cursor blink to off, but allow the terminal to 
+        //    set blinking
         //        "blinking": "terminal_controlled",
         // 3. Always blink the cursor, ignoring the terminal mode
-        //        "blinking": "always",
+        //        "blinking": "on",
         "blinking": "terminal_controlled",
         //Any key-value pairs added to this list will be added to the terminal's
         //enviroment. Use `:` to seperate multiple values.

crates/gpui/src/platform/mac/window.rs 🔗

@@ -1159,7 +1159,6 @@ extern "C" fn first_rect_for_character_range(
         let window = get_window_state(this).borrow().native_window;
         NSView::frame(window)
     };
-
     with_input_handler(this, |input_handler| {
         input_handler.rect_for_range(range.to_range()?)
     })

crates/settings/src/settings.rs 🔗

@@ -89,9 +89,9 @@ pub struct TerminalSettings {
 #[derive(Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)]
 #[serde(rename_all = "snake_case")]
 pub enum TerminalBlink {
-    Never,
+    Off,
     TerminalControlled,
-    Always,
+    On,
 }
 
 impl Default for TerminalBlink {

crates/terminal/src/connected_el.rs 🔗

@@ -645,54 +645,59 @@ impl Element for TerminalEl {
             selection,
         );
 
-        //Layout cursor
-        let cursor = {
-            if self.cursor_visible {
-                let cursor_point = DisplayCursor::from(cursor.point, display_offset);
-                let cursor_text = {
-                    let str_trxt = cursor_text.to_string();
+        //Layout cursor. Rectangle is used for IME, so we should lay it out even
+        //if we don't end up showing it.
+        let cursor = if let alacritty_terminal::ansi::CursorShape::Hidden = cursor.shape {
+            None
+        } else {
+            let cursor_point = DisplayCursor::from(cursor.point, display_offset);
+            let cursor_text = {
+                let str_trxt = cursor_text.to_string();
+
+                let color = if self.focused {
+                    terminal_theme.colors.background
+                } else {
+                    terminal_theme.colors.foreground
+                };
 
-                    let color = if self.focused {
-                        terminal_theme.colors.background
-                    } else {
-                        terminal_theme.colors.foreground
+                cx.text_layout_cache.layout_str(
+                    &str_trxt,
+                    text_style.font_size,
+                    &[(
+                        str_trxt.len(),
+                        RunStyle {
+                            font_id: text_style.font_id,
+                            color,
+                            underline: Default::default(),
+                        },
+                    )],
+                )
+            };
+
+            TerminalEl::shape_cursor(cursor_point, dimensions, &cursor_text).map(
+                move |(cursor_position, block_width)| {
+                    let shape = match cursor.shape {
+                        alacritty_terminal::ansi::CursorShape::Block if !self.focused => {
+                            CursorShape::Hollow
+                        }
+                        alacritty_terminal::ansi::CursorShape::Block => CursorShape::Block,
+                        alacritty_terminal::ansi::CursorShape::Underline => CursorShape::Underscore,
+                        alacritty_terminal::ansi::CursorShape::Beam => CursorShape::Bar,
+                        alacritty_terminal::ansi::CursorShape::HollowBlock => CursorShape::Hollow,
+                        //This case is handled in the wrapping if
+                        alacritty_terminal::ansi::CursorShape::Hidden => CursorShape::Block,
                     };
 
-                    cx.text_layout_cache.layout_str(
-                        &str_trxt,
-                        text_style.font_size,
-                        &[(
-                            str_trxt.len(),
-                            RunStyle {
-                                font_id: text_style.font_id,
-                                color,
-                                underline: Default::default(),
-                            },
-                        )],
+                    Cursor::new(
+                        cursor_position,
+                        block_width,
+                        dimensions.line_height,
+                        terminal_theme.colors.cursor,
+                        shape,
+                        Some(cursor_text),
                     )
-                };
-
-                TerminalEl::shape_cursor(cursor_point, dimensions, &cursor_text).map(
-                    move |(cursor_position, block_width)| {
-                        let shape = if self.focused {
-                            CursorShape::Block
-                        } else {
-                            CursorShape::Hollow
-                        };
-
-                        Cursor::new(
-                            cursor_position,
-                            block_width,
-                            dimensions.line_height,
-                            terminal_theme.colors.cursor,
-                            shape,
-                            Some(cursor_text),
-                        )
-                    },
-                )
-            } else {
-                None
-            }
+                },
+            )
         };
 
         //Done!
@@ -783,10 +788,12 @@ impl Element for TerminalEl {
             });
 
             //Draw cursor
-            if let Some(cursor) = &layout.cursor {
-                cx.paint_layer(clip_bounds, |cx| {
-                    cursor.paint(origin, cx);
-                })
+            if self.cursor_visible {
+                if let Some(cursor) = &layout.cursor {
+                    cx.paint_layer(clip_bounds, |cx| {
+                        cursor.paint(origin, cx);
+                    })
+                }
             }
         });
     }

crates/terminal/src/connected_view.rs 🔗

@@ -141,13 +141,17 @@ impl ConnectedView {
     }
 
     fn show_character_palette(&mut self, _: &ShowCharacterPalette, cx: &mut ViewContext<Self>) {
-        if self
+        if !self
             .terminal
             .read(cx)
             .last_mode
             .contains(TermMode::ALT_SCREEN)
         {
             cx.show_character_palette();
+        } else {
+            self.terminal
+                .read(cx)
+                .try_keystroke(&Keystroke::parse("ctrl-cmd-space").unwrap());
         }
     }
 
@@ -156,8 +160,6 @@ impl ConnectedView {
         cx.notify();
     }
 
-    //2 -> Character palette shows up! But it's incorrectly positioned
-
     pub fn should_show_cursor(
         &self,
         focused: bool,
@@ -187,9 +189,9 @@ impl ConnectedView {
 
         match setting {
             //If the user requested to never blink, don't blink it.
-            TerminalBlink::Never => true,
+            TerminalBlink::Off => true,
             //If the terminal is controlling it, check terminal mode
-            TerminalBlink::TerminalControlled | TerminalBlink::Always => self.blink_state,
+            TerminalBlink::TerminalControlled | TerminalBlink::On => self.blink_state,
         }
     }
 

crates/terminal/src/terminal.rs 🔗

@@ -297,11 +297,8 @@ impl TerminalBuilder {
         let mut term = Term::new(&config, &initial_size, ZedListener(events_tx.clone()));
 
         //Start off blinking if we need to
-        match blink_settings {
-            None | Some(TerminalBlink::TerminalControlled) | Some(TerminalBlink::Always) => {
-                term.set_mode(alacritty_terminal::ansi::Mode::BlinkingCursor)
-            }
-            _ => {}
+        if let Some(TerminalBlink::On) = blink_settings {
+            term.set_mode(alacritty_terminal::ansi::Mode::BlinkingCursor)
         }
 
         let term = Arc::new(FairMutex::new(term));