Refactor mode indicator to remove itself

One of the problems we had is that the status_bar shows a gap between
items, and we want to not add an additional gap for an invisible status
indicator.

crates/theme/src/theme.rs

@@ -402,7 +402,7 @@ pub struct StatusBar {
     pub height: f32,
     pub item_spacing: f32,
     pub cursor_position: TextStyle,
-    pub vim_mode: TextStyle,
+    pub vim_mode_indicator: TextStyle,
     pub active_language: Interactive<ContainedText>,
     pub auto_update_progress_message: TextStyle,
     pub auto_update_done_message: TextStyle,

crates/vim/src/mode_indicator.rs

@@ -1,30 +1,18 @@
-use gpui::{
-    elements::{Empty, Label},
-    AnyElement, Element, Entity, View, ViewContext,
-};
+use gpui::{elements::Label, AnyElement, Element, Entity, View, ViewContext};
 use workspace::{item::ItemHandle, StatusItemView};
 
-use crate::{state::Mode, Vim};
+use crate::state::Mode;
 
 pub struct ModeIndicator {
-    mode: Option<Mode>,
+    pub mode: Mode,
 }
 
 impl ModeIndicator {
-    pub fn new(cx: &mut ViewContext<Self>) -> Self {
-        cx.observe_global::<Vim, _>(|this, cx| {
-            let vim = Vim::read(cx);
-            if vim.enabled {
-                this.set_mode(Some(Vim::read(cx).state.mode), cx)
-            } else {
-                this.set_mode(None, cx)
-            }
-        })
-        .detach();
-        Self { mode: None }
+    pub fn new(mode: Mode) -> Self {
+        Self { mode }
     }
 
-    pub fn set_mode(&mut self, mode: Option<Mode>, cx: &mut ViewContext<Self>) {
+    pub fn set_mode(&mut self, mode: Mode, cx: &mut ViewContext<Self>) {
         if mode != self.mode {
             self.mode = mode;
             cx.notify();
@@ -38,22 +26,21 @@ impl Entity for ModeIndicator {
 
 impl View for ModeIndicator {
     fn ui_name() -> &'static str {
-        "ModeIndicator"
+        "ModeIndicatorView"
     }
 
     fn render(&mut self, cx: &mut ViewContext<Self>) -> AnyElement<Self> {
-        if let Some(mode) = self.mode {
-            let theme = &theme::current(cx).workspace.status_bar;
-            let text = match mode {
-                Mode::Normal => "",
-                Mode::Insert => "--- INSERT ---",
-                Mode::Visual { line: false } => "--- VISUAL ---",
-                Mode::Visual { line: true } => "--- VISUAL LINE ---",
-            };
-            Label::new(text, theme.vim_mode.clone()).into_any()
-        } else {
-            Empty::new().into_any()
-        }
+        let theme = &theme::current(cx).workspace.status_bar;
+        // we always choose text to be 12 monospace characters
+        // so that as the mode indicator changes, the rest of the
+        // UI stays still.
+        let text = match self.mode {
+            Mode::Normal => "-- NORMAL --",
+            Mode::Insert => "-- INSERT --",
+            Mode::Visual { line: false } => "-- VISUAL --",
+            Mode::Visual { line: true } => "VISUAL LINE ",
+        };
+        Label::new(text, theme.vim_mode_indicator.clone()).into_any()
     }
 }
 

crates/vim/src/test.rs

@@ -4,6 +4,8 @@ mod neovim_connection;
 mod vim_binding_test_context;
 mod vim_test_context;
 
+use std::sync::Arc;
+
 use command_palette::CommandPalette;
 use editor::DisplayPoint;
 pub use neovim_backed_binding_test_context::*;
@@ -14,7 +16,7 @@ pub use vim_test_context::*;
 use indoc::indoc;
 use search::BufferSearchBar;
 
-use crate::state::Mode;
+use crate::{state::Mode, ModeIndicator};
 
 #[gpui::test]
 async fn test_initially_disabled(cx: &mut gpui::TestAppContext) {
@@ -195,3 +197,57 @@ async fn test_selection_on_search(cx: &mut gpui::TestAppContext) {
     cx.simulate_keystrokes(["shift-n"]);
     cx.assert_state(indoc! {"aa\nbb\nˇcc\ncc\ncc\n"}, Mode::Normal);
 }
+
+#[gpui::test]
+async fn test_status_indicator(
+    cx: &mut gpui::TestAppContext,
+    deterministic: Arc<gpui::executor::Deterministic>,
+) {
+    let mut cx = VimTestContext::new(cx, true).await;
+    deterministic.run_until_parked();
+
+    let mode_indicator = cx.workspace(|workspace, cx| {
+        let status_bar = workspace.status_bar().read(cx);
+        let mode_indicator = status_bar.item_of_type::<ModeIndicator>();
+        assert!(mode_indicator.is_some());
+        mode_indicator.unwrap()
+    });
+
+    assert_eq!(
+        cx.workspace(|_, cx| mode_indicator.read(cx).mode),
+        Mode::Normal
+    );
+
+    // shows the correct mode
+    cx.simulate_keystrokes(["i"]);
+    deterministic.run_until_parked();
+    assert_eq!(
+        cx.workspace(|_, cx| mode_indicator.read(cx).mode),
+        Mode::Insert
+    );
+
+    // shows even in search
+    cx.simulate_keystrokes(["escape", "v", "/"]);
+    deterministic.run_until_parked();
+    assert_eq!(
+        cx.workspace(|_, cx| mode_indicator.read(cx).mode),
+        Mode::Visual { line: false }
+    );
+
+    // hides if vim mode is disabled
+    cx.disable_vim();
+    deterministic.run_until_parked();
+    cx.workspace(|workspace, cx| {
+        let status_bar = workspace.status_bar().read(cx);
+        let mode_indicator = status_bar.item_of_type::<ModeIndicator>();
+        assert!(mode_indicator.is_none());
+    });
+
+    cx.enable_vim();
+    deterministic.run_until_parked();
+    cx.workspace(|workspace, cx| {
+        let status_bar = workspace.status_bar().read(cx);
+        let mode_indicator = status_bar.item_of_type::<ModeIndicator>();
+        assert!(mode_indicator.is_some());
+    });
+}

crates/vim/src/vim.rs

@@ -118,6 +118,7 @@ pub fn observe_keystrokes(cx: &mut WindowContext) {
 pub struct Vim {
     active_editor: Option<WeakViewHandle<Editor>>,
     editor_subscription: Option<Subscription>,
+    mode_indicator: Option<ViewHandle<ModeIndicator>>,
 
     enabled: bool,
     state: VimState,
@@ -177,6 +178,10 @@ impl Vim {
         self.state.mode = mode;
         self.state.operator_stack.clear();
 
+        if let Some(mode_indicator) = &self.mode_indicator {
+            mode_indicator.update(cx, |mode_indicator, cx| mode_indicator.set_mode(mode, cx))
+        }
+
         // Sync editor settings like clip mode
         self.sync_vim_settings(cx);
 
@@ -259,6 +264,51 @@ impl Vim {
         }
     }
 
+    fn sync_mode_indicator(cx: &mut AppContext) {
+        cx.spawn(|mut cx| async move {
+            let workspace = match cx.update(|cx| {
+                cx.update_active_window(|cx| {
+                    cx.root_view()
+                        .downcast_ref::<Workspace>()
+                        .map(|workspace| workspace.downgrade())
+                })
+            }) {
+                Some(Some(workspace)) => workspace,
+                _ => {
+                    return Ok(());
+                }
+            };
+
+            workspace.update(&mut cx, |workspace, cx| {
+                Vim::update(cx, |vim, cx| {
+                    workspace.status_bar().update(cx, |status_bar, cx| {
+                        let current_position = status_bar.position_of_item::<ModeIndicator>();
+                        if vim.enabled && current_position.is_none() {
+                            if vim.mode_indicator.is_none() {
+                                vim.mode_indicator =
+                                    Some(cx.add_view(|_| ModeIndicator::new(vim.state.mode)));
+                            };
+                            let mode_indicator = vim.mode_indicator.as_ref().unwrap();
+                            // TODO: would it be better to depend on the diagnostics crate
+                            // so we can pass the type directly?
+                            let position = status_bar.position_of_named_item("DiagnosticIndicator");
+                            if let Some(position) = position {
+                                status_bar.insert_item_after(position, mode_indicator.clone(), cx)
+                            } else {
+                                status_bar.add_left_item(mode_indicator.clone(), cx)
+                            }
+                        } else if !vim.enabled {
+                            if let Some(position) = current_position {
+                                status_bar.remove_item_at(position, cx)
+                            }
+                        }
+                    })
+                })
+            })
+        })
+        .detach_and_log_err(cx);
+    }
+
     fn set_enabled(&mut self, enabled: bool, cx: &mut AppContext) {
         if self.enabled != enabled {
             self.enabled = enabled;
@@ -309,6 +359,8 @@ impl Vim {
                 self.unhook_vim_settings(editor, cx);
             }
         });
+
+        Vim::sync_mode_indicator(cx);
     }
 
     fn unhook_vim_settings(&self, editor: &mut Editor, cx: &mut ViewContext<Editor>) {

crates/workspace/src/status_bar.rs

@@ -1,4 +1,4 @@
-use std::{any::TypeId, ops::Range};
+use std::ops::Range;
 
 use crate::{ItemHandle, Pane};
 use gpui::{
@@ -96,14 +96,21 @@ impl StatusBar {
         cx.notify();
     }
 
-    pub fn position_of_item<T>(&mut self) -> Option<usize>
+    pub fn position_of_item<T>(&self) -> Option<usize>
     where
         T: StatusItemView,
     {
         self.position_of_named_item(T::ui_name())
     }
 
-    pub fn position_of_named_item(&mut self, name: &str) -> Option<usize> {
+    pub fn item_of_type<T: StatusItemView>(&self) -> Option<ViewHandle<T>> {
+        self.left_items
+            .iter()
+            .chain(self.right_items.iter())
+            .find_map(|item| item.as_any().clone().downcast())
+    }
+
+    pub fn position_of_named_item(&self, name: &str) -> Option<usize> {
         for (index, item) in self.left_items.iter().enumerate() {
             if item.as_ref().ui_name() == name {
                 return Some(index);
@@ -126,10 +133,10 @@ impl StatusBar {
         T: 'static + StatusItemView,
     {
         if position < self.left_items.len() {
-            self.left_items.insert(position, Box::new(item))
+            self.left_items.insert(position + 1, Box::new(item))
         } else {
             self.right_items
-                .insert(position - self.left_items.len(), Box::new(item))
+                .insert(position + 1 - self.left_items.len(), Box::new(item))
         }
         cx.notify()
     }

crates/zed/src/zed.rs

@@ -312,11 +312,10 @@ pub fn initialize_workspace(
                 feedback::deploy_feedback_button::DeployFeedbackButton::new(workspace)
             });
             let cursor_position = cx.add_view(|_| editor::items::CursorPosition::new());
-            let vim_mode = cx.add_view(|cx| vim::ModeIndicator::new(cx));
             workspace.status_bar().update(cx, |status_bar, cx| {
                 status_bar.add_left_item(diagnostic_summary, cx);
-                status_bar.add_left_item(vim_mode, cx);
                 status_bar.add_left_item(activity_indicator, cx);
+
                 status_bar.add_right_item(feedback_button, cx);
                 status_bar.add_right_item(copilot, cx);
                 status_bar.add_right_item(active_buffer_language, cx);

styles/src/style_tree/status_bar.ts

@@ -27,7 +27,7 @@ export default function status_bar(): any {
         },
         border: border(layer, { top: true, overlay: true }),
         cursor_position: text(layer, "sans", "variant"),
-        vim_mode: text(layer, "sans", "variant"),
+        vim_mode_indicator: text(layer, "mono", "variant"),
         active_language: interactive({
             base: {
                 padding: { left: 6, right: 6 },