From 0410b2340c62eb2ceb7c844aa7fe6a9d21415d5a Mon Sep 17 00:00:00 2001 From: Dino Date: Mon, 15 Dec 2025 19:18:18 +0000 Subject: [PATCH] editor: Refactor cursor_offset_on_selection field in favor of VimModeSettings (#44889) In a previous Pull Request, a new field was added to `editor::Editor`, namely `cursor_offset_on_selection`, in order to control whether the cursor representing the head of a selection should be positioned in the last selected character, as we have on Vim mode, or after, like we have when Vim mode is disabled. This field would then be set by the `vim` crate, depending on the current vim mode. However, it was noted that `vim_mode_setting::VimModeSetting` already exsits and allows other crates to determine whether Vim mode is enabled or not. Since we're already checking `!range.is_empty()` in `editor::element::SelectionLayout::new` we can then rely on simply determining whether Vim mode is enabled to decide whether tho shift the cursor one position to the left when making a selection. As such, this commit removes the `cursor_offset_on_selection` field, as well as any related methods in favor of a new `Editor.vim_mode_enabled` method, which can be used to achieve the same behavior. Relates to #42837 Release Notes: - N/A --- crates/editor/src/editor.rs | 31 +++++++++++++------------------ crates/editor/src/element.rs | 27 ++++++++++++++++----------- crates/vim/src/vim.rs | 1 - 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index bea7d79779b3a1f8ae0473e26235a2a992f7b030..3a6fc630e650ecfbd6f95cf0df30ac9f0228f050 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -202,6 +202,7 @@ use ui::{ IconSize, Indicator, Key, Tooltip, h_flex, prelude::*, scrollbars::ScrollbarAutoHide, }; use util::{RangeExt, ResultExt, TryFutureExt, maybe, post_inc}; +use vim_mode_setting::VimModeSetting; use workspace::{ CollaboratorId, Item as WorkspaceItem, ItemId, ItemNavHistory, OpenInTerminal, OpenTerminal, RestoreOnStartupBehavior, SERIALIZATION_THROTTLE_TIME, SplitDirection, TabBarSettings, Toast, @@ -1110,9 +1111,6 @@ pub struct Editor { pending_rename: Option, searchable: bool, cursor_shape: CursorShape, - /// Whether the cursor is offset one character to the left when something is - /// selected (needed for vim visual mode) - cursor_offset_on_selection: bool, current_line_highlight: Option, pub collapse_matches: bool, autoindent_mode: Option, @@ -2288,7 +2286,6 @@ impl Editor { cursor_shape: EditorSettings::get_global(cx) .cursor_shape .unwrap_or_default(), - cursor_offset_on_selection: false, current_line_highlight: None, autoindent_mode: Some(AutoindentMode::EachLine), collapse_matches: false, @@ -2475,10 +2472,7 @@ impl Editor { } } EditorEvent::Edited { .. } => { - let vim_mode = vim_mode_setting::VimModeSetting::try_get(cx) - .map(|vim_mode| vim_mode.0) - .unwrap_or(false); - if !vim_mode { + if !editor.is_vim_mode_enabled(cx) { let display_map = editor.display_snapshot(cx); let selections = editor.selections.all_adjusted_display(&display_map); let pop_state = editor @@ -3107,10 +3101,6 @@ impl Editor { self.cursor_shape } - pub fn set_cursor_offset_on_selection(&mut self, set_cursor_offset_on_selection: bool) { - self.cursor_offset_on_selection = set_cursor_offset_on_selection; - } - pub fn set_current_line_highlight( &mut self, current_line_highlight: Option, @@ -22607,10 +22597,7 @@ impl Editor { .and_then(|e| e.to_str()) .map(|a| a.to_string())); - let vim_mode = vim_mode_setting::VimModeSetting::try_get(cx) - .map(|vim_mode| vim_mode.0) - .unwrap_or(false); - + let vim_mode_enabled = self.is_vim_mode_enabled(cx); let edit_predictions_provider = all_language_settings(file, cx).edit_predictions.provider; let copilot_enabled = edit_predictions_provider == language::language_settings::EditPredictionProvider::Copilot; @@ -22628,7 +22615,7 @@ impl Editor { event_type, type = if auto_saved {"autosave"} else {"manual"}, file_extension, - vim_mode, + vim_mode_enabled, copilot_enabled, copilot_enabled_for_language, edit_predictions_provider, @@ -22638,7 +22625,7 @@ impl Editor { telemetry::event!( event_type, file_extension, - vim_mode, + vim_mode_enabled, copilot_enabled, copilot_enabled_for_language, edit_predictions_provider, @@ -23253,6 +23240,14 @@ impl Editor { show_underlines: self.diagnostics_enabled(), } } + + /// Returns the value of the `vim_mode` setting, defaulting `false` if the + /// setting is not set. + pub(crate) fn is_vim_mode_enabled(&self, cx: &App) -> bool { + VimModeSetting::try_get(cx) + .map(|vim_mode| vim_mode.0) + .unwrap_or(false) + } } fn edit_for_markdown_paste<'a>( diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 8de660275ba9b455aec610568c41347888654495..efb0459b15b7b7e19a485a81753d39d7dd20b5de 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -133,7 +133,7 @@ impl SelectionLayout { fn new( selection: Selection, line_mode: bool, - cursor_offset: bool, + vim_mode_enabled: bool, cursor_shape: CursorShape, map: &DisplaySnapshot, is_newest: bool, @@ -154,7 +154,7 @@ impl SelectionLayout { } // any vim visual mode (including line mode) - if cursor_offset && !range.is_empty() && !selection.reversed { + if vim_mode_enabled && !range.is_empty() && !selection.reversed { if head.column() > 0 { head = map.clip_point(DisplayPoint::new(head.row(), head.column() - 1), Bias::Left); } else if head.row().0 > 0 && head != map.max_point() { @@ -1463,7 +1463,7 @@ impl EditorElement { let layout = SelectionLayout::new( selection, editor.selections.line_mode(), - editor.cursor_offset_on_selection, + editor.is_vim_mode_enabled(cx), editor.cursor_shape, &snapshot.display_snapshot, is_newest, @@ -1510,7 +1510,7 @@ impl EditorElement { let drag_cursor_layout = SelectionLayout::new( drop_cursor.clone(), false, - editor.cursor_offset_on_selection, + editor.is_vim_mode_enabled(cx), CursorShape::Bar, &snapshot.display_snapshot, false, @@ -1574,7 +1574,7 @@ impl EditorElement { .push(SelectionLayout::new( selection.selection, selection.line_mode, - editor.cursor_offset_on_selection, + editor.is_vim_mode_enabled(cx), selection.cursor_shape, &snapshot.display_snapshot, false, @@ -1585,8 +1585,7 @@ impl EditorElement { selections.extend(remote_selections.into_values()); } else if !editor.is_focused(window) && editor.show_cursor_when_unfocused { - let cursor_offset_on_selection = editor.cursor_offset_on_selection; - + let player = editor.current_user_player_color(cx); let layouts = snapshot .buffer_snapshot() .selections_in_range(&(start_anchor..end_anchor), true) @@ -1594,7 +1593,7 @@ impl EditorElement { SelectionLayout::new( selection, line_mode, - cursor_offset_on_selection, + editor.is_vim_mode_enabled(cx), cursor_shape, &snapshot.display_snapshot, false, @@ -1603,7 +1602,7 @@ impl EditorElement { ) }) .collect::>(); - let player = editor.current_user_player_color(cx); + selections.push((player, layouts)); } }); @@ -3318,7 +3317,7 @@ impl EditorElement { SelectionLayout::new( newest, editor.selections.line_mode(), - editor.cursor_offset_on_selection, + editor.is_vim_mode_enabled(cx), editor.cursor_shape, &snapshot.display_snapshot, true, @@ -11549,6 +11548,7 @@ mod tests { use log::info; use std::num::NonZeroU32; use util::test::sample_text; + use vim_mode_setting::VimModeSetting; #[gpui::test] async fn test_soft_wrap_editor_width_auto_height_editor(cx: &mut TestAppContext) { @@ -11893,6 +11893,12 @@ mod tests { async fn test_vim_visual_selections(cx: &mut TestAppContext) { init_test(cx, |_| {}); + // Enable `vim_mode` setting so the logic that checks whether this is + // enabled can work as expected. + cx.update(|cx| { + VimModeSetting::override_global(VimModeSetting(true), cx); + }); + let window = cx.add_window(|window, cx| { let buffer = MultiBuffer::build_simple(&(sample_text(6, 6, 'a') + "\n"), cx); Editor::new(EditorMode::full(), buffer, None, window, cx) @@ -11903,7 +11909,6 @@ mod tests { window .update(cx, |editor, window, cx| { - editor.cursor_offset_on_selection = true; editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| { s.select_ranges([ Point::new(0, 0)..Point::new(1, 0), diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 26fec968fb261fbb80a9f84211357623147ca0f4..9a9a1a001c32fcf8b22892ce5300d8d2aec3dd37 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -1943,7 +1943,6 @@ impl Vim { editor.set_collapse_matches(collapse_matches); editor.set_input_enabled(vim.editor_input_enabled()); editor.set_autoindent(vim.should_autoindent()); - editor.set_cursor_offset_on_selection(vim.mode.is_visual()); editor .selections .set_line_mode(matches!(vim.mode, Mode::VisualLine));