Skip to content

Commit

Permalink
vim: Improve lifecycle (zed-industries#16477)
Browse files Browse the repository at this point in the history
Closes zed-industries#13579

A major painpoint in the Vim crate has been life-cycle management. We
used to have one global Vim instance that tried to track per-editor
state; this led to a number of subtle issues (e.g. zed-industries#13579, the mode
indicator being global, and quick toggling between windows letting vim
mode's notion of the active editor get out of sync).

This PR changes the internal structure of the code so that there is now
one `Vim` instance per `Editor` (stored as an `Addon`); and the global
stuff is separated out. This fixes the above problems, and tidies up a
bunch of the mess in the codebase.

Release Notes:

* vim: Fixed accidental visual mode in project search and go to
references
([zed-industries#13579](zed-industries#13579)).
  • Loading branch information
ConradIrwin authored Aug 21, 2024
1 parent c4c0758 commit 36d51fe
Show file tree
Hide file tree
Showing 32 changed files with 3,292 additions and 3,515 deletions.
59 changes: 35 additions & 24 deletions crates/editor/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,14 @@ struct ResolvedTasks {
struct MultiBufferOffset(usize);
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
struct BufferOffset(usize);

// Addons allow storing per-editor state in other crates (e.g. Vim)
pub trait Addon: 'static {
fn extend_key_context(&self, _: &mut KeyContext, _: &AppContext) {}

fn to_any(&self) -> &dyn std::any::Any;
}

/// Zed's primary text input `View`, allowing users to edit a [`MultiBuffer`]
///
/// See the [module level documentation](self) for more information.
Expand Down Expand Up @@ -533,7 +541,6 @@ pub struct Editor {
collapse_matches: bool,
autoindent_mode: Option<AutoindentMode>,
workspace: Option<(WeakView<Workspace>, Option<WorkspaceId>)>,
keymap_context_layers: BTreeMap<TypeId, KeyContext>,
input_enabled: bool,
use_modal_editing: bool,
read_only: bool,
Expand All @@ -551,7 +558,6 @@ pub struct Editor {
_subscriptions: Vec<Subscription>,
pixel_position_of_newest_cursor: Option<gpui::Point<Pixels>>,
gutter_dimensions: GutterDimensions,
pub vim_replace_map: HashMap<Range<usize>, String>,
style: Option<EditorStyle>,
next_editor_action_id: EditorActionId,
editor_actions: Rc<RefCell<BTreeMap<EditorActionId, Box<dyn Fn(&mut ViewContext<Self>)>>>>,
Expand Down Expand Up @@ -581,6 +587,7 @@ pub struct Editor {
breadcrumb_header: Option<String>,
focused_block: Option<FocusedBlock>,
next_scroll_position: NextScrollCursorCenterTopBottom,
addons: HashMap<TypeId, Box<dyn Addon>>,
_scroll_cursor_center_top_bottom_task: Task<()>,
}

Expand Down Expand Up @@ -1875,7 +1882,6 @@ impl Editor {
autoindent_mode: Some(AutoindentMode::EachLine),
collapse_matches: false,
workspace: None,
keymap_context_layers: Default::default(),
input_enabled: true,
use_modal_editing: mode == EditorMode::Full,
read_only: false,
Expand All @@ -1900,7 +1906,6 @@ impl Editor {
hovered_cursors: Default::default(),
next_editor_action_id: EditorActionId::default(),
editor_actions: Rc::default(),
vim_replace_map: Default::default(),
show_inline_completions: mode == EditorMode::Full,
custom_context_menu: None,
show_git_blame_gutter: false,
Expand Down Expand Up @@ -1939,6 +1944,7 @@ impl Editor {
breadcrumb_header: None,
focused_block: None,
next_scroll_position: NextScrollCursorCenterTopBottom::default(),
addons: HashMap::default(),
_scroll_cursor_center_top_bottom_task: Task::ready(()),
};
this.tasks_update_task = Some(this.refresh_runnables(cx));
Expand All @@ -1961,13 +1967,13 @@ impl Editor {
this
}

pub fn mouse_menu_is_focused(&self, cx: &mut WindowContext) -> bool {
pub fn mouse_menu_is_focused(&self, cx: &WindowContext) -> bool {
self.mouse_context_menu
.as_ref()
.is_some_and(|menu| menu.context_menu.focus_handle(cx).is_focused(cx))
}

fn key_context(&self, cx: &AppContext) -> KeyContext {
fn key_context(&self, cx: &ViewContext<Self>) -> KeyContext {
let mut key_context = KeyContext::new_with_defaults();
key_context.add("Editor");
let mode = match self.mode {
Expand Down Expand Up @@ -1998,8 +2004,13 @@ impl Editor {
}
}

for layer in self.keymap_context_layers.values() {
key_context.extend(layer);
// Disable vim contexts when a sub-editor (e.g. rename/inline assistant) is focused.
if !self.focus_handle(cx).contains_focused(cx)
|| (self.is_focused(cx) || self.mouse_menu_is_focused(cx))
{
for addon in self.addons.values() {
addon.extend_key_context(&mut key_context, cx)
}
}

if let Some(extension) = self
Expand Down Expand Up @@ -2241,21 +2252,6 @@ impl Editor {
}
}

pub fn set_keymap_context_layer<Tag: 'static>(
&mut self,
context: KeyContext,
cx: &mut ViewContext<Self>,
) {
self.keymap_context_layers
.insert(TypeId::of::<Tag>(), context);
cx.notify();
}

pub fn remove_keymap_context_layer<Tag: 'static>(&mut self, cx: &mut ViewContext<Self>) {
self.keymap_context_layers.remove(&TypeId::of::<Tag>());
cx.notify();
}

pub fn set_input_enabled(&mut self, input_enabled: bool) {
self.input_enabled = input_enabled;
}
Expand Down Expand Up @@ -11864,7 +11860,6 @@ impl Editor {
self.editor_actions.borrow_mut().insert(
id,
Box::new(move |cx| {
let _view = cx.view().clone();
let cx = cx.window_context();
let listener = listener.clone();
cx.on_action(TypeId::of::<A>(), move |action, phase, cx| {
Expand Down Expand Up @@ -11950,6 +11945,22 @@ impl Editor {
menu.visible() && matches!(menu, ContextMenu::Completions(_))
})
}

pub fn register_addon<T: Addon>(&mut self, instance: T) {
self.addons
.insert(std::any::TypeId::of::<T>(), Box::new(instance));
}

pub fn unregister_addon<T: Addon>(&mut self) {
self.addons.remove(&std::any::TypeId::of::<T>());
}

pub fn addon<T: Addon>(&self) -> Option<&T> {
let type_id = std::any::TypeId::of::<T>();
self.addons
.get(&type_id)
.and_then(|item| item.to_any().downcast_ref::<T>())
}
}

fn hunks_for_selections(
Expand Down
2 changes: 1 addition & 1 deletion crates/editor/src/element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5613,7 +5613,7 @@ impl Element for EditorElement {
cx: &mut WindowContext,
) {
let focus_handle = self.editor.focus_handle(cx);
let key_context = self.editor.read(cx).key_context(cx);
let key_context = self.editor.update(cx, |editor, cx| editor.key_context(cx));
cx.set_key_context(key_context);
cx.handle_input(
&focus_handle,
Expand Down
110 changes: 50 additions & 60 deletions crates/vim/src/change_list.rs
Original file line number Diff line number Diff line change
@@ -1,64 +1,55 @@
use editor::{display_map::ToDisplayPoint, movement, scroll::Autoscroll, Bias, Direction, Editor};
use gpui::{actions, View};
use ui::{ViewContext, WindowContext};
use workspace::Workspace;
use gpui::{actions, ViewContext};

use crate::{state::Mode, Vim};

actions!(vim, [ChangeListOlder, ChangeListNewer]);

pub(crate) fn register(workspace: &mut Workspace, _: &mut ViewContext<Workspace>) {
workspace.register_action(|_, _: &ChangeListOlder, cx| {
Vim::update(cx, |vim, cx| {
move_to_change(vim, Direction::Prev, cx);
})
pub(crate) fn register(editor: &mut Editor, cx: &mut ViewContext<Vim>) {
Vim::action(editor, cx, |vim, _: &ChangeListOlder, cx| {
vim.move_to_change(Direction::Prev, cx);
});
workspace.register_action(|_, _: &ChangeListNewer, cx| {
Vim::update(cx, |vim, cx| {
move_to_change(vim, Direction::Next, cx);
})
Vim::action(editor, cx, |vim, _: &ChangeListNewer, cx| {
vim.move_to_change(Direction::Next, cx);
});
}

fn move_to_change(vim: &mut Vim, direction: Direction, cx: &mut WindowContext) {
let count = vim.take_count(cx).unwrap_or(1);
let selections = vim.update_state(|state| {
if state.change_list.is_empty() {
return None;
impl Vim {
fn move_to_change(&mut self, direction: Direction, cx: &mut ViewContext<Self>) {
let count = self.take_count(cx).unwrap_or(1);
if self.change_list.is_empty() {
return;
}

let prev = state
.change_list_position
.unwrap_or(state.change_list.len());
let prev = self.change_list_position.unwrap_or(self.change_list.len());
let next = if direction == Direction::Prev {
prev.saturating_sub(count)
} else {
(prev + count).min(state.change_list.len() - 1)
(prev + count).min(self.change_list.len() - 1)
};
state.change_list_position = Some(next);
state.change_list.get(next).cloned()
});

let Some(selections) = selections else {
return;
};
vim.update_active_editor(cx, |_, editor, cx| {
editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
let map = s.display_map();
s.select_display_ranges(selections.into_iter().map(|a| {
let point = a.to_display_point(&map);
point..point
}))
})
});
}
self.change_list_position = Some(next);
let Some(selections) = self.change_list.get(next).cloned() else {
return;
};
self.update_editor(cx, |_, editor, cx| {
editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
let map = s.display_map();
s.select_display_ranges(selections.into_iter().map(|a| {
let point = a.to_display_point(&map);
point..point
}))
})
});
}

pub(crate) fn push_to_change_list(vim: &mut Vim, editor: View<Editor>, cx: &mut WindowContext) {
let (map, selections) =
editor.update(cx, |editor, cx| editor.selections.all_adjusted_display(cx));
pub(crate) fn push_to_change_list(&mut self, cx: &mut ViewContext<Self>) {
let Some((map, selections)) = self.update_editor(cx, |_, editor, cx| {
editor.selections.all_adjusted_display(cx)
}) else {
return;
};

let pop_state =
vim.state()
let pop_state = self
.change_list
.last()
.map(|previous| {
Expand All @@ -69,25 +60,24 @@ pub(crate) fn push_to_change_list(vim: &mut Vim, editor: View<Editor>, cx: &mut
})
.unwrap_or(false);

let new_positions = selections
.into_iter()
.map(|s| {
let point = if vim.state().mode == Mode::Insert {
movement::saturating_left(&map, s.head())
} else {
s.head()
};
map.display_point_to_anchor(point, Bias::Left)
})
.collect();

vim.update_state(|state| {
state.change_list_position.take();
let new_positions = selections
.into_iter()
.map(|s| {
let point = if self.mode == Mode::Insert {
movement::saturating_left(&map, s.head())
} else {
s.head()
};
map.display_point_to_anchor(point, Bias::Left)
})
.collect();

self.change_list_position.take();
if pop_state {
state.change_list.pop();
self.change_list.pop();
}
state.change_list.push(new_positions);
})
self.change_list.push(new_positions);
}
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 36d51fe

Please sign in to comment.