Fix bug with IME

Adjust how IME works in the terminal

co-authored-by: nathan <nathan@zed.dev>

crates/gpui2/src/platform.rs

@@ -145,6 +145,7 @@ pub trait PlatformWindow {
     fn mouse_position(&self) -> Point<Pixels>;
     fn as_any_mut(&mut self) -> &mut dyn Any;
     fn set_input_handler(&mut self, input_handler: Box<dyn PlatformInputHandler>);
+    fn clear_input_handler(&mut self);
     fn prompt(&self, level: PromptLevel, msg: &str, answers: &[&str]) -> oneshot::Receiver<usize>;
     fn activate(&self);
     fn set_title(&mut self, title: &str);

crates/gpui2/src/platform/mac/window.rs

@@ -750,6 +750,10 @@ impl PlatformWindow for MacWindow {
         self.0.as_ref().lock().input_handler = Some(input_handler);
     }
 
+    fn clear_input_handler(&mut self) {
+        self.0.as_ref().lock().input_handler = None;
+    }
+
     fn prompt(&self, level: PromptLevel, msg: &str, answers: &[&str]) -> oneshot::Receiver<usize> {
         // macOs applies overrides to modal window buttons after they are added.
         // Two most important for this logic are:

crates/gpui2/src/platform/test/window.rs

@@ -89,6 +89,10 @@ impl PlatformWindow for TestWindow {
         self.input_handler = Some(Arc::new(Mutex::new(input_handler)));
     }
 
+    fn clear_input_handler(&mut self) {
+        self.input_handler = None;
+    }
+
     fn prompt(
         &self,
         _level: crate::PromptLevel,

crates/gpui2/src/window.rs

@@ -1231,6 +1231,7 @@ impl<'a> WindowContext<'a> {
     /// Rotate the current frame and the previous frame, then clear the current frame.
     /// We repopulate all state in the current frame during each paint.
     fn start_frame(&mut self) {
+        self.window.platform_window.clear_input_handler();
         self.text_system().start_frame();
 
         let window = &mut *self.window;

crates/terminal_view2/src/terminal_element.rs

@@ -1,11 +1,11 @@
 use editor::{Cursor, HighlightedRange, HighlightedRangeLine};
 use gpui::{
-    black, div, point, px, red, relative, transparent_black, AnyElement, AvailableSpace, Bounds,
+    black, div, point, px, red, relative, transparent_black, AnyElement, AsyncWindowContext,
-    DispatchPhase, Element, ElementId, ElementInputHandler, FocusHandle, Font, FontStyle,
+    AvailableSpace, Bounds, DispatchPhase, Element, ElementId, FocusHandle, Font, FontStyle,
     FontWeight, HighlightStyle, Hsla, InteractiveElement, InteractiveElementState, IntoElement,
-    LayoutId, ModelContext, ModifiersChangedEvent, MouseButton, Pixels, Point, Rgba, ShapedLine,
+    LayoutId, Model, ModelContext, ModifiersChangedEvent, MouseButton, Pixels,
-    Size, StatefulInteractiveElement, Styled, TextRun, TextStyle, TextSystem, UnderlineStyle, View,
+    PlatformInputHandler, Point, Rgba, ShapedLine, Size, StatefulInteractiveElement, Styled,
-    WeakModel, WhiteSpace, WindowContext,
+    TextRun, TextStyle, TextSystem, UnderlineStyle, View, WhiteSpace, WindowContext,
 };
 use itertools::Itertools;
 use language::CursorShape;
@@ -148,7 +148,7 @@ impl LayoutRect {
 ///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 TerminalElement {
-    terminal: WeakModel<Terminal>,
+    terminal: Model<Terminal>,
     terminal_view: View<TerminalView>,
     focus: FocusHandle,
     focused: bool,
@@ -167,7 +167,7 @@ impl StatefulInteractiveElement for TerminalElement {}
 
 impl TerminalElement {
     pub fn new(
-        terminal: WeakModel<Terminal>,
+        terminal: Model<Terminal>,
         terminal_view: View<TerminalView>,
         focus: FocusHandle,
         focused: bool,
@@ -461,16 +461,11 @@ impl TerminalElement {
             TerminalSize::new(line_height, cell_width, size)
         };
 
-        let search_matches = if let Some(terminal_model) = self.terminal.upgrade() {
+        let search_matches = self.terminal.read(cx).matches.clone();
-            terminal_model.read(cx).matches.clone()
-        } else {
-            Default::default()
-        };
 
         let background_color = theme.colors().background;
-        let terminal_handle = self.terminal.upgrade().unwrap();
 
-        let last_hovered_word = terminal_handle.update(cx, |terminal, cx| {
+        let last_hovered_word = self.terminal.update(cx, |terminal, cx| {
             terminal.set_size(dimensions);
             terminal.try_sync(cx);
             if self.can_navigate_to_selected_word && terminal.can_navigate_to_selected_word() {
@@ -495,7 +490,7 @@ impl TerminalElement {
             selection,
             cursor,
             ..
-        } = &terminal_handle.read(cx).last_content;
+        } = &self.terminal.read(cx).last_content;
 
         // searches, highlights to a single range representations
         let mut relative_highlighted_ranges = Vec::new();
@@ -592,20 +587,18 @@ impl TerminalElement {
     }
 
     fn generic_button_handler<E>(
-        connection: WeakModel<Terminal>,
+        connection: Model<Terminal>,
         origin: Point<Pixels>,
         focus_handle: FocusHandle,
         f: impl Fn(&mut Terminal, Point<Pixels>, &E, &mut ModelContext<Terminal>),
     ) -> impl Fn(&E, &mut WindowContext) {
         move |event, cx| {
             cx.focus(&focus_handle);
-            if let Some(conn_handle) = connection.upgrade() {
+            connection.update(cx, |terminal, cx| {
-                conn_handle.update(cx, |terminal, cx| {
+                f(terminal, origin, event, cx);
-                    f(terminal, origin, event, cx);
 
-                    cx.notify();
+                cx.notify();
-                })
+            })
-            }
         }
     }
 
@@ -617,10 +610,10 @@ impl TerminalElement {
                     return;
                 }
 
-                let handled = this
+                let handled =
-                    .update(cx, |term, _| term.try_modifiers_change(&event.modifiers))
+                    this.update(cx, |term, _| term.try_modifiers_change(&event.modifiers));
-                    .ok();
+
-                if handled == Some(true) {
+                if handled {
                     cx.notify();
                 }
             }
@@ -645,13 +638,11 @@ impl TerminalElement {
                     cx.focus(&focus);
                     //todo!(context menu)
                     // v.context_menu.update(cx, |menu, _cx| menu.delay_cancel());
-                    if let Some(conn_handle) = connection.upgrade() {
+                    connection.update(cx, |terminal, cx| {
-                        conn_handle.update(cx, |terminal, cx| {
+                        terminal.mouse_down(&e, origin);
-                            terminal.mouse_down(&e, origin);
 
-                            cx.notify();
+                        cx.notify();
-                        })
+                    })
-                    }
                 }
             })
             .on_mouse_move({
@@ -660,12 +651,10 @@ impl TerminalElement {
                 move |e, cx| {
                     if e.pressed_button.is_some() {
                         if focus.is_focused(cx) {
-                            if let Some(conn_handle) = connection.upgrade() {
+                            connection.update(cx, |terminal, cx| {
-                                conn_handle.update(cx, |terminal, cx| {
+                                terminal.mouse_drag(e, origin, bounds);
-                                    terminal.mouse_drag(e, origin, bounds);
+                                cx.notify();
-                                    cx.notify();
+                            })
-                                })
-                            }
                         }
                     }
                 }
@@ -685,14 +674,10 @@ impl TerminalElement {
                 let connection = connection.clone();
                 move |e, cx| {
                     if e.down.button == MouseButton::Right {
-                        let mouse_mode = if let Some(conn_handle) = connection.upgrade() {
+                        let mouse_mode = connection.update(cx, |terminal, _cx| {
-                            conn_handle.update(cx, |terminal, _cx| {
+                            terminal.mouse_mode(e.down.modifiers.shift)
-                                terminal.mouse_mode(e.down.modifiers.shift)
+                        });
-                            })
+
-                        } else {
-                            // If we can't get the model handle, probably can't deploy the context menu
-                            true
-                        };
                         if !mouse_mode {
                             //todo!(context menu)
                             // view.deploy_context_menu(e.position, cx);
@@ -705,24 +690,20 @@ impl TerminalElement {
                 let focus = focus.clone();
                 move |e, cx| {
                     if focus.is_focused(cx) {
-                        if let Some(conn_handle) = connection.upgrade() {
+                        connection.update(cx, |terminal, cx| {
-                            conn_handle.update(cx, |terminal, cx| {
+                            terminal.mouse_move(&e, origin);
-                                terminal.mouse_move(&e, origin);
+                            cx.notify();
-                                cx.notify();
+                        })
-                            })
-                        }
                     }
                 }
             })
             .on_scroll_wheel({
                 let connection = connection.clone();
                 move |e, cx| {
-                    if let Some(conn_handle) = connection.upgrade() {
+                    connection.update(cx, |terminal, cx| {
-                        conn_handle.update(cx, |terminal, cx| {
+                        terminal.scroll_wheel(e, origin);
-                            terminal.scroll_wheel(e, origin);
+                        cx.notify();
-                            cx.notify();
+                    })
-                        })
-                    }
                 }
             });
 
@@ -822,13 +803,21 @@ impl Element for TerminalElement {
         );
         let origin = bounds.origin + Point::new(layout.gutter, px(0.));
 
+        let terminal_input_handler = TerminalInputHandler {
+            cx: cx.to_async(),
+            terminal: self.terminal.clone(),
+            cursor_bounds: layout
+                .cursor
+                .as_ref()
+                .map(|cursor| cursor.bounding_rect(origin)),
+        };
+
         let mut this = self.register_mouse_listeners(origin, layout.mode, bounds, cx);
 
         let interactivity = mem::take(&mut this.interactivity);
 
         interactivity.paint(bounds, bounds.size, state, cx, |_, _, cx| {
-            let input_handler = ElementInputHandler::new(bounds, this.terminal_view.clone(), cx);
+            cx.handle_input(&this.focus, terminal_input_handler);
-            cx.handle_input(&this.focus, input_handler);
 
             this.register_key_listeners(cx);
 
@@ -890,6 +879,69 @@ impl IntoElement for TerminalElement {
     }
 }
 
+struct TerminalInputHandler {
+    cx: AsyncWindowContext,
+    terminal: Model<Terminal>,
+    cursor_bounds: Option<Bounds<Pixels>>,
+}
+
+impl PlatformInputHandler for TerminalInputHandler {
+    fn selected_text_range(&mut self) -> Option<std::ops::Range<usize>> {
+        self.cx
+            .update(|_, cx| {
+                if self
+                    .terminal
+                    .read(cx)
+                    .last_content
+                    .mode
+                    .contains(TermMode::ALT_SCREEN)
+                {
+                    None
+                } else {
+                    Some(0..0)
+                }
+            })
+            .ok()
+            .flatten()
+    }
+
+    fn marked_text_range(&mut self) -> Option<std::ops::Range<usize>> {
+        None
+    }
+
+    fn text_for_range(&mut self, range_utf16: std::ops::Range<usize>) -> Option<String> {
+        None
+    }
+
+    fn replace_text_in_range(
+        &mut self,
+        _replacement_range: Option<std::ops::Range<usize>>,
+        text: &str,
+    ) {
+        self.cx
+            .update(|_, cx| {
+                self.terminal.update(cx, |terminal, _| {
+                    terminal.input(text.into());
+                })
+            })
+            .ok();
+    }
+
+    fn replace_and_mark_text_in_range(
+        &mut self,
+        _range_utf16: Option<std::ops::Range<usize>>,
+        _new_text: &str,
+        _new_selected_range: Option<std::ops::Range<usize>>,
+    ) {
+    }
+
+    fn unmark_text(&mut self) {}
+
+    fn bounds_for_range(&mut self, _range_utf16: std::ops::Range<usize>) -> Option<Bounds<Pixels>> {
+        self.cursor_bounds
+    }
+}
+
 fn is_blank(cell: &IndexedCell) -> bool {
     if cell.c != ' ' {
         return false;

crates/terminal_view2/src/terminal_view.rs

@@ -9,10 +9,9 @@ pub mod terminal_panel;
 // use crate::terminal_element::TerminalElement;
 use editor::{scroll::autoscroll::Autoscroll, Editor};
 use gpui::{
-    actions, div, point, px, size, Action, AnyElement, AppContext, Bounds, Div, EventEmitter,
+    actions, div, Action, AnyElement, AppContext, Div, EventEmitter, FocusEvent, FocusHandle,
-    FocusEvent, FocusHandle, Focusable, FocusableElement, FocusableView, Font, FontStyle,
+    Focusable, FocusableElement, FocusableView, KeyContext, KeyDownEvent, Keystroke, Model,
-    FontWeight, InputHandler, KeyContext, KeyDownEvent, Keystroke, Model, MouseButton,
+    MouseButton, MouseDownEvent, Pixels, Render, Subscription, Task, View, VisualContext, WeakView,
-    MouseDownEvent, Pixels, Render, Task, View, VisualContext, WeakView, Subscription
 };
 use language::Bias;
 use persistence::TERMINAL_DB;
@@ -26,7 +25,6 @@ use terminal::{
     Event, MaybeNavigationTarget, Terminal,
 };
 use terminal_element::TerminalElement;
-use theme::ThemeSettings;
 use ui::{h_stack, prelude::*, ContextMenu, Icon, IconElement, Label};
 use util::{paths::PathLikeWithPosition, ResultExt};
 use workspace::{
@@ -624,7 +622,7 @@ impl Render for TerminalView {
     type Element = Focusable<Div>;
 
     fn render(&mut self, cx: &mut ViewContext<Self>) -> Self::Element {
-        let terminal_handle = self.terminal.clone().downgrade();
+        let terminal_handle = self.terminal.clone();
         let this_view = cx.view().clone();
 
         let self_id = cx.entity_id();
@@ -673,124 +671,6 @@ impl Render for TerminalView {
     }
 }
 
-//todo!(Implement IME)
-impl InputHandler for TerminalView {
-    fn text_for_range(
-        &mut self,
-        range: std::ops::Range<usize>,
-        cx: &mut ViewContext<Self>,
-    ) -> Option<String> {
-        None
-    }
-
-    fn selected_text_range(
-        &mut self,
-        cx: &mut ViewContext<Self>,
-    ) -> Option<std::ops::Range<usize>> {
-        if self
-            .terminal
-            .read(cx)
-            .last_content
-            .mode
-            .contains(TermMode::ALT_SCREEN)
-        {
-            None
-        } else {
-            Some(0..0)
-        }
-    }
-
-    fn marked_text_range(&self, _cx: &mut ViewContext<Self>) -> Option<std::ops::Range<usize>> {
-        None
-    }
-
-    fn unmark_text(&mut self, _cx: &mut ViewContext<Self>) {}
-
-    fn replace_text_in_range(
-        &mut self,
-        _: Option<std::ops::Range<usize>>,
-        text: &str,
-        cx: &mut ViewContext<Self>,
-    ) {
-        self.terminal.update(cx, |terminal, _| {
-            terminal.input(text.into());
-        });
-    }
-
-    fn replace_and_mark_text_in_range(
-        &mut self,
-        _range: Option<std::ops::Range<usize>>,
-        _new_text: &str,
-        _new_selected_range: Option<std::ops::Range<usize>>,
-        _cx: &mut ViewContext<Self>,
-    ) {
-    }
-
-    // todo!(Check that this works correctly, why aren't we reading the range?)
-    fn bounds_for_range(
-        &mut self,
-        _range_utf16: std::ops::Range<usize>,
-        bounds: gpui::Bounds<Pixels>,
-        cx: &mut ViewContext<Self>,
-    ) -> Option<gpui::Bounds<Pixels>> {
-        let settings = ThemeSettings::get_global(cx).clone();
-
-        let buffer_font_size = settings.buffer_font_size(cx);
-
-        let terminal_settings = TerminalSettings::get_global(cx);
-        let font_family = terminal_settings
-            .font_family
-            .as_ref()
-            .map(|string| string.clone().into())
-            .unwrap_or(settings.buffer_font.family);
-
-        let line_height = terminal_settings
-            .line_height
-            .value()
-            .to_pixels(cx.rem_size());
-
-        let font_size = terminal_settings.font_size.clone();
-        let features = terminal_settings
-            .font_features
-            .clone()
-            .unwrap_or(settings.buffer_font.features.clone());
-
-        let font_size =
-            font_size.map_or(buffer_font_size, |size| theme::adjusted_font_size(size, cx));
-
-        let font_id = cx
-            .text_system()
-            .font_id(&Font {
-                family: font_family,
-                style: FontStyle::Normal,
-                weight: FontWeight::NORMAL,
-                features,
-            })
-            .unwrap();
-
-        let cell_width = cx
-            .text_system()
-            .advance(font_id, font_size, 'm')
-            .unwrap()
-            .width;
-
-        let mut origin = bounds.origin + point(cell_width, px(0.));
-
-        // TODO - Why is it necessary to move downward one line to get correct
-        // positioning? I would think that we'd want the same rect that is
-        // painted for the cursor.
-        origin += point(px(0.), line_height);
-
-        let cursor = Bounds {
-            origin,
-            //todo!(correctly calculate this width and height based on the text the line is over)
-            size: size(cell_width, line_height),
-        };
-
-        Some(cursor)
-    }
-}
-
 impl Item for TerminalView {
     type Event = ItemEvent;