From 43d94e37eccd15ac5fad02811989682c387728b6 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 24 Jul 2023 09:37:48 -0600 Subject: [PATCH] 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 | 2 +- crates/vim/src/mode_indicator.rs | 49 +++++++++--------------- crates/vim/src/test.rs | 58 ++++++++++++++++++++++++++++- crates/vim/src/vim.rs | 52 ++++++++++++++++++++++++++ crates/workspace/src/status_bar.rs | 17 ++++++--- crates/zed/src/zed.rs | 3 +- styles/src/style_tree/status_bar.ts | 2 +- 7 files changed, 142 insertions(+), 41 deletions(-) diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index de0701a343..82c3f2a142 100644 --- a/crates/theme/src/theme.rs +++ b/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, pub auto_update_progress_message: TextStyle, pub auto_update_done_message: TextStyle, diff --git a/crates/vim/src/mode_indicator.rs b/crates/vim/src/mode_indicator.rs index ef8d159018..683024267c 100644 --- a/crates/vim/src/mode_indicator.rs +++ b/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, + pub mode: Mode, } impl ModeIndicator { - pub fn new(cx: &mut ViewContext) -> Self { - cx.observe_global::(|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, cx: &mut ViewContext) { + pub fn set_mode(&mut self, mode: Mode, cx: &mut ViewContext) { 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) -> AnyElement { - 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() } } diff --git a/crates/vim/src/test.rs b/crates/vim/src/test.rs index 8ed649e61b..96d6a2b690 100644 --- a/crates/vim/src/test.rs +++ b/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, +) { + 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::(); + 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::(); + 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::(); + assert!(mode_indicator.is_some()); + }); +} diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 8ab110ba1e..54d18825cd 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -118,6 +118,7 @@ pub fn observe_keystrokes(cx: &mut WindowContext) { pub struct Vim { active_editor: Option>, editor_subscription: Option, + mode_indicator: Option>, 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::() + .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::(); + 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) { diff --git a/crates/workspace/src/status_bar.rs b/crates/workspace/src/status_bar.rs index 6fd3bd5310..7b1c11dcf2 100644 --- a/crates/workspace/src/status_bar.rs +++ b/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(&mut self) -> Option + pub fn position_of_item(&self) -> Option where T: StatusItemView, { self.position_of_named_item(T::ui_name()) } - pub fn position_of_named_item(&mut self, name: &str) -> Option { + pub fn item_of_type(&self) -> Option> { + 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 { 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() } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 645371d419..639e1a3f60 100644 --- a/crates/zed/src/zed.rs +++ b/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); diff --git a/styles/src/style_tree/status_bar.ts b/styles/src/style_tree/status_bar.ts index b4273cbf99..74ad7064d1 100644 --- a/styles/src/style_tree/status_bar.ts +++ b/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 },