Fixed blink problems

Mikayla Maki created

Change summary

assets/settings/default.json          | 11 +---
crates/editor/src/element.rs          | 55 +++++--------------------
crates/settings/src/settings.rs       |  5 -
crates/terminal/src/connected_el.rs   | 60 ++++++----------------------
crates/terminal/src/connected_view.rs | 53 +++++++++++++++++++++----
crates/terminal/src/terminal.rs       | 21 +++++----
6 files changed, 86 insertions(+), 119 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -106,15 +106,12 @@
         //May take 4 values:
         // 1. Never blink the cursor, ignoring the terminal mode
         //        "blinking": "never",
-        // 2. Default the cursor blink to off, but allow the terminal to 
-        //    turn blinking on
-        //        "blinking": "off",
-        // 3. Default the cursor blink to on, but allow the terminal to 
+        // 2. Default the cursor blink to on, but allow the terminal to 
         //    turn blinking off
-        //        "blinking": "on",
-        // 4. Always blink the cursor, ignoring the terminal mode
+        //        "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.
         "env": {

crates/editor/src/element.rs 🔗

@@ -1801,7 +1801,7 @@ impl Cursor {
     pub fn paint(&self, origin: Vector2F, cx: &mut PaintContext) {
         let bounds = match self.shape {
             CursorShape::Bar => RectF::new(self.origin + origin, vec2f(2.0, self.line_height)),
-            CursorShape::Block => RectF::new(
+            CursorShape::Block | CursorShape::Hollow => RectF::new(
                 self.origin + origin,
                 vec2f(self.block_width, self.line_height),
             ),
@@ -1809,58 +1809,27 @@ impl Cursor {
                 self.origin + origin + Vector2F::new(0.0, self.line_height - 2.0),
                 vec2f(self.block_width, 2.0),
             ),
-            CursorShape::Hollow => RectF::new(
-                self.origin + origin + Vector2F::new(0.0, self.line_height - 1.0),
-                vec2f(self.block_width, 1.0),
-            ),
         };
 
-        //Draw text under the hollow block if need be
+        //Draw background or border quad
         if matches!(self.shape, CursorShape::Hollow) {
-            if let Some(block_text) = &self.block_text {
-                block_text.paint(self.origin + origin, bounds, self.line_height, cx);
-            }
-        }
-
-        cx.scene.push_quad(Quad {
-            bounds,
-            background: Some(self.color),
-            border: Border::new(0., Color::black()),
-            corner_radius: 0.,
-        });
-
-        if matches!(self.shape, CursorShape::Hollow) {
-            //Top
-            cx.scene.push_quad(Quad {
-                bounds: RectF::new(
-                    self.origin + origin + Vector2F::new(0.0, -1.0),
-                    vec2f(self.block_width + 1., 1.0),
-                ),
-                background: Some(self.color),
-                border: Border::new(0., Color::black()),
-                corner_radius: 0.,
-            });
-            //Left
             cx.scene.push_quad(Quad {
-                bounds: RectF::new(self.origin + origin, vec2f(1.0, self.line_height)),
-                background: Some(self.color),
-                border: Border::new(0., Color::black()),
+                bounds,
+                background: None,
+                border: Border::all(1., self.color),
                 corner_radius: 0.,
             });
-            //Right
+        } else {
             cx.scene.push_quad(Quad {
-                bounds: RectF::new(
-                    self.origin + origin + vec2f(self.block_width, 0.),
-                    vec2f(1.0, self.line_height),
-                ),
+                bounds,
                 background: Some(self.color),
-                border: Border::new(0., Color::black()),
+                border: Default::default(),
                 corner_radius: 0.,
             });
-        } else {
-            if let Some(block_text) = &self.block_text {
-                block_text.paint(self.origin + origin, bounds, self.line_height, cx);
-            }
+        }
+
+        if let Some(block_text) = &self.block_text {
+            block_text.paint(self.origin + origin, bounds, self.line_height, cx);
         }
     }
 }

crates/settings/src/settings.rs 🔗

@@ -90,14 +90,13 @@ pub struct TerminalSettings {
 #[serde(rename_all = "snake_case")]
 pub enum TerminalBlink {
     Never,
-    On,
-    Off,
+    TerminalControlled,
     Always,
 }
 
 impl Default for TerminalBlink {
     fn default() -> Self {
-        TerminalBlink::On
+        TerminalBlink::TerminalControlled
     }
 }
 

crates/terminal/src/connected_el.rs 🔗

@@ -21,7 +21,7 @@ use gpui::{
 };
 use itertools::Itertools;
 use ordered_float::OrderedFloat;
-use settings::{Settings, TerminalBlink};
+use settings::Settings;
 use theme::TerminalStyle;
 use util::ResultExt;
 
@@ -201,7 +201,7 @@ pub struct TerminalEl {
     view: WeakViewHandle<ConnectedView>,
     modal: bool,
     focused: bool,
-    blink_state: bool,
+    cursor_visible: bool,
 }
 
 impl TerminalEl {
@@ -210,14 +210,14 @@ impl TerminalEl {
         terminal: WeakModelHandle<Terminal>,
         modal: bool,
         focused: bool,
-        blink_state: bool,
+        cursor_visible: bool,
     ) -> TerminalEl {
         TerminalEl {
             view,
             terminal,
             modal,
             focused,
-            blink_state,
+            cursor_visible,
         }
     }
 
@@ -571,33 +571,6 @@ impl TerminalEl {
 
         (point, side)
     }
-
-    pub fn should_show_cursor(
-        settings: Option<TerminalBlink>,
-        blinking_on: bool,
-        focused: bool,
-        blink_show: bool,
-    ) -> bool {
-        if !focused {
-            true
-        } else {
-            match settings {
-                Some(setting) => match setting {
-                TerminalBlink::Never => true,
-                TerminalBlink::On | TerminalBlink::Off if blinking_on => blink_show,
-                TerminalBlink::On | TerminalBlink::Off /*if !blinking_on */ => true,
-                TerminalBlink::Always => focused && blink_show,
-            },
-                None => {
-                    if blinking_on {
-                        blink_show
-                    } else {
-                        false
-                    }
-                }
-            }
-        }
-    }
 }
 
 impl Element for TerminalEl {
@@ -610,7 +583,6 @@ impl Element for TerminalEl {
         cx: &mut gpui::LayoutContext,
     ) -> (gpui::geometry::vector::Vector2F, Self::LayoutState) {
         let settings = cx.global::<Settings>();
-        let blink_settings = settings.terminal_overrides.blinking.clone();
         let font_cache = cx.font_cache();
 
         //Setup layout information
@@ -629,13 +601,13 @@ impl Element for TerminalEl {
             terminal_theme.colors.background
         };
 
-        let (cells, selection, cursor, display_offset, cursor_text, blink_mode) = self
+        let (cells, selection, cursor, display_offset, cursor_text) = self
             .terminal
             .upgrade(cx)
             .unwrap()
             .update(cx.app, |terminal, mcx| {
                 terminal.set_size(dimensions);
-                terminal.render_lock(mcx, |content, cursor_text, blink_mode| {
+                terminal.render_lock(mcx, |content, cursor_text| {
                     let mut cells = vec![];
                     cells.extend(
                         content
@@ -659,7 +631,6 @@ impl Element for TerminalEl {
                         content.cursor,
                         content.display_offset,
                         cursor_text,
-                        blink_mode,
                     )
                 })
             });
@@ -676,14 +647,7 @@ impl Element for TerminalEl {
 
         //Layout cursor
         let cursor = {
-            if !TerminalEl::should_show_cursor(
-                blink_settings,
-                blink_mode,
-                self.focused,
-                self.blink_state,
-            ) {
-                None
-            } else {
+            if self.cursor_visible {
                 let cursor_point = DisplayCursor::from(cursor.point, display_offset);
                 let cursor_text = {
                     let str_trxt = cursor_text.to_string();
@@ -710,22 +674,24 @@ impl Element for TerminalEl {
 
                 TerminalEl::shape_cursor(cursor_point, dimensions, &cursor_text).map(
                     move |(cursor_position, block_width)| {
-                        let (shape, color) = if self.focused {
-                            (CursorShape::Block, terminal_theme.colors.cursor)
+                        let shape = if self.focused {
+                            CursorShape::Block
                         } else {
-                            (CursorShape::Hollow, terminal_theme.colors.foreground)
+                            CursorShape::Hollow
                         };
 
                         Cursor::new(
                             cursor_position,
                             block_width,
                             dimensions.line_height,
-                            color,
+                            terminal_theme.colors.cursor,
                             shape,
                             Some(cursor_text),
                         )
                     },
                 )
+            } else {
+                None
             }
         };
 

crates/terminal/src/connected_view.rs 🔗

@@ -11,6 +11,7 @@ use gpui::{
     AnyViewHandle, AppContext, Element, ElementBox, ModelHandle, MutableAppContext, View,
     ViewContext, ViewHandle,
 };
+use settings::{Settings, TerminalBlink};
 use smol::Timer;
 use workspace::pane;
 
@@ -67,7 +68,8 @@ pub struct ConnectedView {
     // Only for styling purposes. Doesn't effect behavior
     modal: bool,
     context_menu: ViewHandle<ContextMenu>,
-    show_cursor: bool,
+    blink_state: bool,
+    blinking_on: bool,
     blinking_paused: bool,
     blink_epoch: usize,
 }
@@ -91,7 +93,7 @@ impl ConnectedView {
                 this.has_bell = true;
                 cx.emit(Event::Wakeup);
             }
-
+            Event::BlinkChanged => this.blinking_on = !this.blinking_on,
             _ => cx.emit(*event),
         })
         .detach();
@@ -102,7 +104,8 @@ impl ConnectedView {
             has_bell: false,
             modal,
             context_menu: cx.add_view(ContextMenu::new),
-            show_cursor: true,
+            blink_state: true,
+            blinking_on: false,
             blinking_paused: false,
             blink_epoch: 0,
         }
@@ -153,14 +156,46 @@ impl ConnectedView {
         cx.notify();
     }
 
-    //Following code copied from editor cursor
-    pub fn blink_show(&self) -> bool {
-        self.blinking_paused || self.show_cursor
+    //2 -> Character palette shows up! But it's incorrectly positioned
+
+    pub fn should_show_cursor(
+        &self,
+        focused: bool,
+        cx: &mut gpui::RenderContext<'_, Self>,
+    ) -> bool {
+        //Don't blink the cursor when not focused, blinking is disabled, or paused
+        if !focused
+            || !self.blinking_on
+            || self.blinking_paused
+            || self
+                .terminal
+                .read(cx)
+                .last_mode
+                .contains(TermMode::ALT_SCREEN)
+        {
+            return true;
+        }
+
+        let setting = {
+            let settings = cx.global::<Settings>();
+            settings
+                .terminal_overrides
+                .blinking
+                .clone()
+                .unwrap_or(TerminalBlink::TerminalControlled)
+        };
+
+        match setting {
+            //If the user requested to never blink, don't blink it.
+            TerminalBlink::Never => true,
+            //If the terminal is controlling it, check terminal mode
+            TerminalBlink::TerminalControlled | TerminalBlink::Always => self.blink_state,
+        }
     }
 
     fn blink_cursors(&mut self, epoch: usize, cx: &mut ViewContext<Self>) {
         if epoch == self.blink_epoch && !self.blinking_paused {
-            self.show_cursor = !self.show_cursor;
+            self.blink_state = !self.blink_state;
             cx.notify();
 
             let epoch = self.next_blink_epoch();
@@ -178,7 +213,7 @@ impl ConnectedView {
     }
 
     pub fn pause_cursor_blinking(&mut self, cx: &mut ViewContext<Self>) {
-        self.show_cursor = true;
+        self.blink_state = true;
         cx.notify();
 
         let epoch = self.next_blink_epoch();
@@ -280,7 +315,7 @@ impl View for ConnectedView {
                     terminal_handle,
                     self.modal,
                     focused,
-                    self.blink_show(),
+                    self.should_show_cursor(focused, cx),
                 )
                 .contained()
                 .boxed(),

crates/terminal/src/terminal.rs 🔗

@@ -19,6 +19,8 @@ use alacritty_terminal::{
 };
 use anyhow::{bail, Result};
 
+//When you type a key, scroll does not happen in terminal !!!TODO
+
 use futures::{
     channel::mpsc::{unbounded, UnboundedReceiver, UnboundedSender},
     FutureExt,
@@ -62,6 +64,7 @@ pub enum Event {
     CloseTerminal,
     Bell,
     Wakeup,
+    BlinkChanged,
 }
 
 #[derive(Clone, Debug)]
@@ -295,14 +298,12 @@ impl TerminalBuilder {
 
         //Start off blinking if we need to
         match blink_settings {
-            Some(setting) => match setting {
-                TerminalBlink::On | TerminalBlink::Always => {
-                    term.set_mode(alacritty_terminal::ansi::Mode::BlinkingCursor)
-                }
-                _ => {}
-            },
-            None => term.set_mode(alacritty_terminal::ansi::Mode::BlinkingCursor),
+            None | Some(TerminalBlink::TerminalControlled) | Some(TerminalBlink::Always) => {
+                term.set_mode(alacritty_terminal::ansi::Mode::BlinkingCursor)
+            }
+            _ => {}
         }
+
         let term = Arc::new(FairMutex::new(term));
 
         //Setup the pty...
@@ -479,7 +480,7 @@ impl Terminal {
                 self.notify_pty(format(self.cur_size.into()))
             }
             AlacTermEvent::CursorBlinkingChange => {
-                //TODO whatever state we need to set to get the cursor blinking
+                cx.emit(Event::BlinkChanged);
             }
             AlacTermEvent::Bell => {
                 cx.emit(Event::Bell);
@@ -595,7 +596,7 @@ impl Terminal {
 
     pub fn render_lock<F, T>(&mut self, cx: &mut ModelContext<Self>, f: F) -> T
     where
-        F: FnOnce(RenderableContent, char, bool) -> T,
+        F: FnOnce(RenderableContent, char) -> T,
     {
         let m = self.term.clone(); //Arc clone
         let mut term = m.lock();
@@ -611,7 +612,7 @@ impl Terminal {
 
         let cursor_text = term.grid()[content.cursor.point].c;
 
-        f(content, cursor_text, term.cursor_style().blinking)
+        f(content, cursor_text)
     }
 
     ///Scroll the terminal