Revert "Keep selection in SwitchToHelixNormalMode (#41583)" (#42892)

Closes #ISSUE

Release Notes:

- Fixes vim "go to definition" making a selection
This commit is contained in:
Conrad Irwin
2025-11-17 11:01:34 -07:00
committed by GitHub
parent 46ad6c0bbb
commit 6bf5e92a25
18 changed files with 94 additions and 183 deletions
-6
View File
@@ -421,12 +421,6 @@
"ctrl-[": "editor::Cancel"
}
},
{
"context": "vim_mode == helix_select && !menu",
"bindings": {
"escape": "vim::SwitchToHelixNormalMode"
}
},
{
"context": "(vim_mode == helix_normal || vim_mode == helix_select) && !menu",
"bindings": {
+1 -2
View File
@@ -2626,12 +2626,11 @@ impl SearchableItem for TextThreadEditor {
&mut self,
index: usize,
matches: &[Self::Match],
collapse: bool,
window: &mut Window,
cx: &mut Context<Self>,
) {
self.editor.update(cx, |editor, cx| {
editor.activate_match(index, matches, collapse, window, cx);
editor.activate_match(index, matches, window, cx);
});
}
+2 -4
View File
@@ -1029,13 +1029,11 @@ impl SearchableItem for DapLogView {
&mut self,
index: usize,
matches: &[Self::Match],
collapse: bool,
window: &mut Window,
cx: &mut Context<Self>,
) {
self.editor.update(cx, |e, cx| {
e.activate_match(index, matches, collapse, window, cx)
})
self.editor
.update(cx, |e, cx| e.activate_match(index, matches, window, cx))
}
fn select_matches(
+16 -32
View File
@@ -1099,6 +1099,7 @@ pub struct Editor {
searchable: bool,
cursor_shape: CursorShape,
current_line_highlight: Option<CurrentLineHighlight>,
collapse_matches: bool,
autoindent_mode: Option<AutoindentMode>,
workspace: Option<(WeakEntity<Workspace>, Option<WorkspaceId>)>,
input_enabled: bool,
@@ -2211,7 +2212,7 @@ impl Editor {
.unwrap_or_default(),
current_line_highlight: None,
autoindent_mode: Some(AutoindentMode::EachLine),
collapse_matches: false,
workspace: None,
input_enabled: !is_minimap,
use_modal_editing: full_mode,
@@ -2384,7 +2385,10 @@ impl Editor {
}
}
EditorEvent::Edited { .. } => {
if vim_flavor(cx).is_none() {
let vim_mode = vim_mode_setting::VimModeSetting::try_get(cx)
.map(|vim_mode| vim_mode.0)
.unwrap_or(false);
if !vim_mode {
let display_map = editor.display_snapshot(cx);
let selections = editor.selections.all_adjusted_display(&display_map);
let pop_state = editor
@@ -3013,12 +3017,12 @@ impl Editor {
self.current_line_highlight = current_line_highlight;
}
pub fn range_for_match<T: std::marker::Copy>(
&self,
range: &Range<T>,
collapse: bool,
) -> Range<T> {
if collapse {
pub fn set_collapse_matches(&mut self, collapse_matches: bool) {
self.collapse_matches = collapse_matches;
}
pub fn range_for_match<T: std::marker::Copy>(&self, range: &Range<T>) -> Range<T> {
if self.collapse_matches {
return range.start..range.start;
}
range.clone()
@@ -16921,7 +16925,7 @@ impl Editor {
editor.update_in(cx, |editor, window, cx| {
let range = target_range.to_point(target_buffer.read(cx));
let range = editor.range_for_match(&range, false);
let range = editor.range_for_match(&range);
let range = collapse_multiline_range(range);
if !split
@@ -21761,7 +21765,9 @@ impl Editor {
.and_then(|e| e.to_str())
.map(|a| a.to_string()));
let vim_mode = vim_flavor(cx).is_some();
let vim_mode = vim_mode_setting::VimModeSetting::try_get(cx)
.map(|vim_mode| vim_mode.0)
.unwrap_or(false);
let edit_predictions_provider = all_language_settings(file, cx).edit_predictions.provider;
let copilot_enabled = edit_predictions_provider
@@ -22396,28 +22402,6 @@ fn edit_for_markdown_paste<'a>(
(range, new_text)
}
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub enum VimFlavor {
Vim,
Helix,
}
pub fn vim_flavor(cx: &App) -> Option<VimFlavor> {
if vim_mode_setting::HelixModeSetting::try_get(cx)
.map(|helix_mode| helix_mode.0)
.unwrap_or(false)
{
Some(VimFlavor::Helix)
} else if vim_mode_setting::VimModeSetting::try_get(cx)
.map(|vim_mode| vim_mode.0)
.unwrap_or(false)
{
Some(VimFlavor::Vim)
} else {
None // neither vim nor helix mode
}
}
fn process_completion_for_edit(
completion: &Completion,
intent: CompletionIntent,
+1 -2
View File
@@ -1586,12 +1586,11 @@ impl SearchableItem for Editor {
&mut self,
index: usize,
matches: &[Range<Anchor>],
collapse: bool,
window: &mut Window,
cx: &mut Context<Self>,
) {
self.unfold_ranges(&[matches[index].clone()], false, true, cx);
let range = self.range_for_match(&matches[index], collapse);
let range = self.range_for_match(&matches[index]);
let autoscroll = if EditorSettings::get_global(cx).search.center_on_match {
Autoscroll::center()
} else {
+2 -4
View File
@@ -812,13 +812,11 @@ impl SearchableItem for LspLogView {
&mut self,
index: usize,
matches: &[Self::Match],
collapse: bool,
window: &mut Window,
cx: &mut Context<Self>,
) {
self.editor.update(cx, |e, cx| {
e.activate_match(index, matches, collapse, window, cx)
})
self.editor
.update(cx, |e, cx| e.activate_match(index, matches, window, cx))
}
fn select_matches(
+7 -14
View File
@@ -10,9 +10,8 @@ use any_vec::AnyVec;
use anyhow::Context as _;
use collections::HashMap;
use editor::{
DisplayPoint, Editor, EditorSettings, VimFlavor,
DisplayPoint, Editor, EditorSettings,
actions::{Backtab, Tab},
vim_flavor,
};
use futures::channel::oneshot;
use gpui::{
@@ -828,8 +827,7 @@ impl BufferSearchBar {
.searchable_items_with_matches
.get(&active_searchable_item.downgrade())
{
let collapse = editor::vim_flavor(cx) == Some(VimFlavor::Vim);
active_searchable_item.activate_match(match_ix, matches, collapse, window, cx)
active_searchable_item.activate_match(match_ix, matches, window, cx)
}
}
@@ -976,8 +974,7 @@ impl BufferSearchBar {
window: &mut Window,
cx: &mut Context<Self>,
) {
let collapse = vim_flavor(cx) == Some(VimFlavor::Vim);
self.select_match(Direction::Next, 1, collapse, window, cx);
self.select_match(Direction::Next, 1, window, cx);
}
fn select_prev_match(
@@ -986,8 +983,7 @@ impl BufferSearchBar {
window: &mut Window,
cx: &mut Context<Self>,
) {
let collapse = vim_flavor(cx) == Some(VimFlavor::Vim);
self.select_match(Direction::Prev, 1, collapse, window, cx);
self.select_match(Direction::Prev, 1, window, cx);
}
pub fn select_all_matches(
@@ -1012,7 +1008,6 @@ impl BufferSearchBar {
&mut self,
direction: Direction,
count: usize,
collapse: bool,
window: &mut Window,
cx: &mut Context<Self>,
) {
@@ -1035,7 +1030,7 @@ impl BufferSearchBar {
.match_index_for_direction(matches, index, direction, count, window, cx);
searchable_item.update_matches(matches, window, cx);
searchable_item.activate_match(new_match_index, matches, collapse, window, cx);
searchable_item.activate_match(new_match_index, matches, window, cx);
}
}
@@ -1049,8 +1044,7 @@ impl BufferSearchBar {
return;
}
searchable_item.update_matches(matches, window, cx);
let collapse = vim_flavor(cx) == Some(VimFlavor::Vim);
searchable_item.activate_match(0, matches, collapse, window, cx);
searchable_item.activate_match(0, matches, window, cx);
}
}
@@ -1065,8 +1059,7 @@ impl BufferSearchBar {
}
let new_match_index = matches.len() - 1;
searchable_item.update_matches(matches, window, cx);
let collapse = vim_flavor(cx) == Some(VimFlavor::Vim);
searchable_item.activate_match(new_match_index, matches, collapse, window, cx);
searchable_item.activate_match(new_match_index, matches, window, cx);
}
}
+3 -6
View File
@@ -9,12 +9,11 @@ use anyhow::Context as _;
use collections::HashMap;
use editor::{
Anchor, Editor, EditorEvent, EditorSettings, MAX_TAB_TITLE_LEN, MultiBuffer, PathKey,
SelectionEffects, VimFlavor,
SelectionEffects,
actions::{Backtab, SelectAll, Tab},
items::active_match_index,
multibuffer_context_lines,
scroll::Autoscroll,
vim_flavor,
};
use futures::{StreamExt, stream::FuturesOrdered};
use gpui::{
@@ -1431,8 +1430,7 @@ impl ProjectSearchView {
let range_to_select = match_ranges[new_index].clone();
self.results_editor.update(cx, |editor, cx| {
let collapse = vim_flavor(cx) == Some(VimFlavor::Vim);
let range_to_select = editor.range_for_match(&range_to_select, collapse);
let range_to_select = editor.range_for_match(&range_to_select);
let autoscroll = if EditorSettings::get_global(cx).search.center_on_match {
Autoscroll::center()
} else {
@@ -1509,10 +1507,9 @@ impl ProjectSearchView {
let is_new_search = self.search_id != prev_search_id;
self.results_editor.update(cx, |editor, cx| {
if is_new_search {
let collapse = vim_flavor(cx) == Some(VimFlavor::Vim);
let range_to_select = match_ranges
.first()
.map(|range| editor.range_for_match(range, collapse));
.map(|range| editor.range_for_match(range));
editor.change_selections(Default::default(), window, cx, |s| {
s.select_ranges(range_to_select)
});
@@ -1452,7 +1452,6 @@ impl SearchableItem for TerminalView {
&mut self,
index: usize,
_: &[Self::Match],
_collapse: bool,
_window: &mut Window,
cx: &mut Context<Self>,
) {
+4 -60
View File
@@ -450,7 +450,7 @@ impl Vim {
prior_selections,
prior_operator: self.operator_stack.last().cloned(),
prior_mode: self.mode,
is_helix_regex_search: true,
helix_select: true,
}
});
}
@@ -1278,24 +1278,6 @@ mod test {
cx.assert_state("«one ˇ»two", Mode::HelixSelect);
}
#[gpui::test]
async fn test_exit_visual_mode(cx: &mut gpui::TestAppContext) {
let mut cx = VimTestContext::new(cx, true).await;
cx.set_state("ˇone two", Mode::Normal);
cx.simulate_keystrokes("v w");
cx.assert_state("«one tˇ»wo", Mode::Visual);
cx.simulate_keystrokes("escape");
cx.assert_state("one ˇtwo", Mode::Normal);
cx.enable_helix();
cx.set_state("ˇone two", Mode::HelixNormal);
cx.simulate_keystrokes("v w");
cx.assert_state("«one ˇ»two", Mode::HelixSelect);
cx.simulate_keystrokes("escape");
cx.assert_state("«one ˇ»two", Mode::HelixNormal);
}
#[gpui::test]
async fn test_helix_select_regex(cx: &mut gpui::TestAppContext) {
let mut cx = VimTestContext::new(cx, true).await;
@@ -1315,47 +1297,9 @@ mod test {
cx.simulate_keystrokes("enter");
cx.assert_state("«oneˇ» two «oneˇ»", Mode::HelixNormal);
// TODO: change "search_in_selection" to not perform any search when in helix select mode with no selection
// cx.set_state("ˇstuff one two one", Mode::HelixNormal);
// cx.simulate_keystrokes("s o n e enter");
// cx.assert_state("ˇstuff one two one", Mode::HelixNormal);
}
#[gpui::test]
async fn test_helix_select_next_match(cx: &mut gpui::TestAppContext) {
let mut cx = VimTestContext::new(cx, true).await;
cx.set_state("ˇhello two one two one two one", Mode::Visual);
cx.simulate_keystrokes("/ o n e");
cx.simulate_keystrokes("enter");
cx.simulate_keystrokes("n n");
cx.assert_state("«hello two one two one two oˇ»ne", Mode::Visual);
cx.set_state("ˇhello two one two one two one", Mode::Normal);
cx.simulate_keystrokes("/ o n e");
cx.simulate_keystrokes("enter");
cx.simulate_keystrokes("n n");
cx.assert_state("hello two one two one two ˇone", Mode::Normal);
cx.set_state("ˇhello two one two one two one", Mode::Normal);
cx.simulate_keystrokes("/ o n e");
cx.simulate_keystrokes("enter");
cx.simulate_keystrokes("n g n g n");
cx.assert_state("hello two one two «one two oneˇ»", Mode::Visual);
cx.enable_helix();
cx.set_state("ˇhello two one two one two one", Mode::HelixNormal);
cx.simulate_keystrokes("/ o n e");
cx.simulate_keystrokes("enter");
cx.simulate_keystrokes("n n");
cx.assert_state("hello two one two one two «oneˇ»", Mode::HelixNormal);
cx.set_state("ˇhello two one two one two one", Mode::HelixSelect);
cx.simulate_keystrokes("/ o n e");
cx.simulate_keystrokes("enter");
cx.simulate_keystrokes("n n");
cx.assert_state("ˇhello two «oneˇ» two «oneˇ» two «oneˇ»", Mode::HelixSelect);
cx.set_state("ˇone two one", Mode::HelixNormal);
cx.simulate_keystrokes("s o n e enter");
cx.assert_state("ˇone two one", Mode::HelixNormal);
}
#[gpui::test]
+18 -27
View File
@@ -672,40 +672,31 @@ pub fn register(editor: &mut Editor, cx: &mut Context<Vim>) {
impl Vim {
pub(crate) fn search_motion(&mut self, m: Motion, window: &mut Window, cx: &mut Context<Self>) {
let Motion::ZedSearchResult {
prior_selections,
new_selections,
if let Motion::ZedSearchResult {
prior_selections, ..
} = &m
else {
return;
};
match self.mode {
Mode::Visual | Mode::VisualLine | Mode::VisualBlock => {
if !prior_selections.is_empty() {
self.update_editor(cx, |_, editor, cx| {
editor.change_selections(Default::default(), window, cx, |s| {
s.select_ranges(prior_selections.iter().cloned());
{
match self.mode {
Mode::Visual | Mode::VisualLine | Mode::VisualBlock => {
if !prior_selections.is_empty() {
self.update_editor(cx, |_, editor, cx| {
editor.change_selections(Default::default(), window, cx, |s| {
s.select_ranges(prior_selections.iter().cloned())
})
});
});
}
}
self.motion(m, window, cx);
}
Mode::Normal | Mode::Replace | Mode::Insert => {
if self.active_operator().is_some() {
self.motion(m, window, cx);
Mode::Normal | Mode::Replace | Mode::Insert => {
if self.active_operator().is_none() {
return;
}
}
}
Mode::HelixNormal => {}
Mode::HelixSelect => {
self.update_editor(cx, |_, editor, cx| {
editor.change_selections(Default::default(), window, cx, |s| {
s.select_ranges(prior_selections.iter().chain(new_selections).cloned());
});
});
Mode::HelixNormal | Mode::HelixSelect => {}
}
}
self.motion(m, window, cx)
}
pub(crate) fn motion(&mut self, motion: Motion, window: &mut Window, cx: &mut Context<Self>) {
+7 -12
View File
@@ -1,6 +1,5 @@
use editor::{Editor, EditorSettings, VimFlavor};
use editor::{Editor, EditorSettings};
use gpui::{Action, Context, Window, actions};
use language::Point;
use schemars::JsonSchema;
use search::{BufferSearchBar, SearchOptions, buffer_search};
@@ -196,7 +195,7 @@ impl Vim {
prior_selections,
prior_operator: self.operator_stack.last().cloned(),
prior_mode,
is_helix_regex_search: false,
helix_select: false,
}
});
}
@@ -220,7 +219,7 @@ impl Vim {
let new_selections = self.editor_selections(window, cx);
let result = pane.update(cx, |pane, cx| {
let search_bar = pane.toolbar().read(cx).item_of_type::<BufferSearchBar>()?;
if self.search.is_helix_regex_search {
if self.search.helix_select {
search_bar.update(cx, |search_bar, cx| {
search_bar.select_all_matches(&Default::default(), window, cx)
});
@@ -241,8 +240,7 @@ impl Vim {
count = count.saturating_sub(1)
}
self.search.count = 1;
let collapse = !self.mode.is_helix();
search_bar.select_match(direction, count, collapse, window, cx);
search_bar.select_match(direction, count, window, cx);
search_bar.focus_editor(&Default::default(), window, cx);
let prior_selections: Vec<_> = self.search.prior_selections.drain(..).collect();
@@ -309,8 +307,7 @@ impl Vim {
if !search_bar.has_active_match() || !search_bar.show(window, cx) {
return false;
}
let collapse = !self.mode.is_helix();
search_bar.select_match(direction, count, collapse, window, cx);
search_bar.select_match(direction, count, window, cx);
true
})
});
@@ -319,7 +316,6 @@ impl Vim {
}
let new_selections = self.editor_selections(window, cx);
self.search_motion(
Motion::ZedSearchResult {
prior_selections,
@@ -385,8 +381,7 @@ impl Vim {
cx.spawn_in(window, async move |_, cx| {
search.await?;
search_bar.update_in(cx, |search_bar, window, cx| {
let collapse = editor::vim_flavor(cx) == Some(VimFlavor::Vim);
search_bar.select_match(direction, count, collapse, window, cx);
search_bar.select_match(direction, count, window, cx);
vim.update(cx, |vim, cx| {
let new_selections = vim.editor_selections(window, cx);
@@ -449,7 +444,7 @@ impl Vim {
cx.spawn_in(window, async move |_, cx| {
search.await?;
search_bar.update_in(cx, |search_bar, window, cx| {
search_bar.select_match(direction, 1, true, window, cx)
search_bar.select_match(direction, 1, window, cx)
})?;
anyhow::Ok(())
})
+2 -6
View File
@@ -67,16 +67,12 @@ impl Display for Mode {
}
impl Mode {
pub fn is_visual(self) -> bool {
pub fn is_visual(&self) -> bool {
match self {
Self::Visual | Self::VisualLine | Self::VisualBlock | Self::HelixSelect => true,
Self::Normal | Self::Insert | Self::Replace | Self::HelixNormal => false,
}
}
pub fn is_helix(self) -> bool {
matches!(self, Mode::HelixNormal | Mode::HelixSelect)
}
}
#[derive(Clone, Debug, PartialEq)]
@@ -991,7 +987,7 @@ pub struct SearchState {
pub prior_selections: Vec<Range<Anchor>>,
pub prior_operator: Option<Operator>,
pub prior_mode: Mode,
pub is_helix_regex_search: bool,
pub helix_select: bool,
}
impl Operator {
+20
View File
@@ -1139,6 +1139,26 @@ async fn test_rename(cx: &mut gpui::TestAppContext) {
cx.assert_state("const afterˇ = 2; console.log(after)", Mode::Normal)
}
#[gpui::test]
async fn test_go_to_definition(cx: &mut gpui::TestAppContext) {
let mut cx = VimTestContext::new_typescript(cx).await;
cx.set_state("const before = 2; console.log(beforˇe)", Mode::Normal);
let def_range = cx.lsp_range("const «beforeˇ» = 2; console.log(before)");
let mut go_to_request =
cx.set_request_handler::<lsp::request::GotoDefinition, _, _>(move |url, _, _| async move {
Ok(Some(lsp::GotoDefinitionResponse::Scalar(
lsp::Location::new(url.clone(), def_range),
)))
});
cx.simulate_keystrokes("g d");
go_to_request.next().await.unwrap();
cx.run_until_parked();
cx.assert_state("const ˇbefore = 2; console.log(before)", Mode::Normal);
}
#[perf]
#[gpui::test]
async fn test_remap(cx: &mut gpui::TestAppContext) {
+1
View File
@@ -59,6 +59,7 @@ impl VimTestContext {
prepare_provider: Some(true),
work_done_progress_options: Default::default(),
})),
definition_provider: Some(lsp::OneOf::Left(true)),
..Default::default()
},
cx,
+3 -1
View File
@@ -668,7 +668,7 @@ impl Vim {
editor,
cx,
|vim, _: &SwitchToHelixNormalMode, window, cx| {
vim.switch_mode(Mode::HelixNormal, true, window, cx)
vim.switch_mode(Mode::HelixNormal, false, window, cx)
},
);
Vim::action(editor, cx, |_, _: &PushForcedMotion, _, cx| {
@@ -954,6 +954,7 @@ impl Vim {
fn deactivate(editor: &mut Editor, cx: &mut Context<Editor>) {
editor.set_cursor_shape(CursorShape::Bar, cx);
editor.set_clip_at_line_ends(false, cx);
editor.set_collapse_matches(false);
editor.set_input_enabled(true);
editor.set_autoindent(true);
editor.selections.set_line_mode(false);
@@ -1929,6 +1930,7 @@ impl Vim {
self.update_editor(cx, |vim, editor, cx| {
editor.set_cursor_shape(vim.cursor_shape(cx), cx);
editor.set_clip_at_line_ends(vim.clip_at_line_ends(), cx);
editor.set_collapse_matches(true);
editor.set_input_enabled(vim.editor_input_enabled());
editor.set_autoindent(vim.should_autoindent());
editor
+6 -2
View File
@@ -847,6 +847,9 @@ impl Vim {
let mut start_selection = 0usize;
let mut end_selection = 0usize;
self.update_editor(cx, |_, editor, _| {
editor.set_collapse_matches(false);
});
if vim_is_normal {
pane.update(cx, |pane, cx| {
if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::<BufferSearchBar>()
@@ -857,7 +860,7 @@ impl Vim {
}
// without update_match_index there is a bug when the cursor is before the first match
search_bar.update_match_index(window, cx);
search_bar.select_match(direction.opposite(), 1, false, window, cx);
search_bar.select_match(direction.opposite(), 1, window, cx);
});
}
});
@@ -875,7 +878,7 @@ impl Vim {
if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::<BufferSearchBar>() {
search_bar.update(cx, |search_bar, cx| {
search_bar.update_match_index(window, cx);
search_bar.select_match(direction, count, false, window, cx);
search_bar.select_match(direction, count, window, cx);
match_exists = search_bar.match_exists(window, cx);
});
}
@@ -902,6 +905,7 @@ impl Vim {
editor.change_selections(Default::default(), window, cx, |s| {
s.select_ranges([start_selection..end_selection]);
});
editor.set_collapse_matches(true);
});
match self.maybe_pop_operator() {
+1 -4
View File
@@ -104,7 +104,6 @@ pub trait SearchableItem: Item + EventEmitter<SearchEvent> {
&mut self,
index: usize,
matches: &[Self::Match],
collapse: bool,
window: &mut Window,
cx: &mut Context<Self>,
);
@@ -186,7 +185,6 @@ pub trait SearchableItemHandle: ItemHandle {
&self,
index: usize,
matches: &AnyVec<dyn Send>,
collapse: bool,
window: &mut Window,
cx: &mut App,
);
@@ -279,13 +277,12 @@ impl<T: SearchableItem> SearchableItemHandle for Entity<T> {
&self,
index: usize,
matches: &AnyVec<dyn Send>,
collapse: bool,
window: &mut Window,
cx: &mut App,
) {
let matches = matches.downcast_ref().unwrap();
self.update(cx, |this, cx| {
this.activate_match(index, matches.as_slice(), collapse, window, cx)
this.activate_match(index, matches.as_slice(), window, cx)
});
}