From d51f669647f1cb7ea1cbd88876040f5c076e928c Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Wed, 14 Feb 2024 11:25:25 +0100 Subject: [PATCH] Vi mode: avoid placing cursor beyond last character Today fish_cursor_selection_mode controls whether selection mode includes the cursor. Since it's by default only used for Vi mode, perhaps use it to also decide whether it should be allowed to select one-past the last character. Not allowing to select to select one-past the last character is much nicer in Vi mode. Unfortunately Vi mode sometimes needs to temporarily select past end (using forward-single-char and such), so reset fish_cursor_selection_mode for the duration of the binding. Also fix other things like cursor placement after yank/yank-pop. Closes #10286 Closes #3299 --- share/functions/fish_vi_key_bindings.fish | 43 ++++++---- src/editable_line.rs | 1 + src/env_dispatch.rs | 26 +++++- src/reader.rs | 97 ++++++++++++++++++----- 4 files changed, 133 insertions(+), 34 deletions(-) diff --git a/share/functions/fish_vi_key_bindings.fish b/share/functions/fish_vi_key_bindings.fish index 94a255996..c18838f32 100644 --- a/share/functions/fish_vi_key_bindings.fish +++ b/share/functions/fish_vi_key_bindings.fish @@ -65,12 +65,12 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' bind -s --preset -M default l forward-char bind -s --preset -m insert \n execute bind -s --preset -m insert \r execute - bind -s --preset -m insert o insert-line-under repaint-mode - bind -s --preset -m insert O insert-line-over repaint-mode + bind -s --preset -m insert o 'set fish_cursor_end_mode exclusive' insert-line-under repaint-mode + bind -s --preset -m insert O 'set fish_cursor_end_modefish_cursor_end_modeexclusive' insert-line-over repaint-mode bind -s --preset -m insert i repaint-mode bind -s --preset -m insert I beginning-of-line repaint-mode - bind -s --preset -m insert a forward-single-char repaint-mode - bind -s --preset -m insert A end-of-line repaint-mode + bind -s --preset -m insert a 'set fish_cursor_end_mode exclusive' forward-single-char repaint-mode + bind -s --preset -m insert A 'set fish_cursor_end_mode exclusive' end-of-line repaint-mode bind -s --preset -m visual v begin-selection repaint-mode bind -s --preset gg beginning-of-buffer @@ -98,8 +98,8 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' bind -s --preset gE backward-bigword bind -s --preset w forward-word forward-single-char bind -s --preset W forward-bigword forward-single-char - bind -s --preset e forward-single-char forward-word backward-char - bind -s --preset E forward-single-char forward-bigword backward-char + bind -s --preset e 'set fish_cursor_end_mode exclusive' forward-single-char forward-word backward-char 'set fish_cursor_end_mode inclusive' + bind -s --preset E 'set fish_cursor_end_mode exclusive' forward-single-char forward-bigword backward-char 'set fish_cursor_end_mode inclusive' bind -s --preset -M insert \cn accept-autosuggestion @@ -112,10 +112,10 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' # Vi moves the cursor back if, after deleting, it is at EOL. # To emulate that, move forward, then backward, which will be a NOP # if there is something to move forward to. - bind -s --preset -M default x delete-char forward-single-char backward-char + bind -s --preset -M default x delete-char 'set fish_cursor_end_mode exclusive' forward-single-char backward-char 'set fish_cursor_end_mode inclusive' bind -s --preset -M default X backward-delete-char bind -s --preset -M insert -k dc delete-char forward-single-char backward-char - bind -s --preset -M default -k dc delete-char forward-single-char backward-char + bind -s --preset -M default -k dc delete-char 'set fish_cursor_end_mode exclusive' forward-single-char backward-char 'set fish_cursor_end_mode inclusive' # Backspace deletes a char in insert mode, but not in normal/default mode. bind -s --preset -M insert -k backspace backward-delete-char @@ -225,13 +225,13 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' # in vim p means paste *after* current character, so go forward a char before pasting # also in vim, P means paste *at* current position (like at '|' with cursor = line), # \ so there's no need to go back a char, just paste it without moving - bind -s --preset p forward-char yank + bind -s --preset p 'set -g fish_cursor_end_mode exclusive' forward-char 'set -g fish_cursor_end_modefish_cursor_end_modeinclusive' yank bind -s --preset P yank bind -s --preset gp yank-pop # same vim 'pasting' note as upper - bind -s --preset '"*p' forward-char "commandline -i ( xsel -p; echo )[1]" - bind -s --preset '"*P' "commandline -i ( xsel -p; echo )[1]" + bind -s --preset '"*p' 'set -g fish_cursor_end_mode exclusive' forward-char 'set -g fish_cursor_end_mode inclusive' fish_clipboard_paste + bind -s --preset '"*P' fish_clipboard_paste # # Lowercase r, enters replace_one mode @@ -267,8 +267,8 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' bind -s --preset -M visual gE backward-bigword bind -s --preset -M visual w forward-word bind -s --preset -M visual W forward-bigword - bind -s --preset -M visual e forward-word - bind -s --preset -M visual E forward-bigword + bind -s --preset -M visual e 'set fish_cursor_end_mode exclusive' forward-single-char forward-word backward-char 'set fish_cursor_end_mode inclusive' + bind -s --preset -M visual E 'set fish_cursor_end_mode exclusive' forward-single-char forward-bigword backward-char 'set fish_cursor_end_mode inclusive' bind -s --preset -M visual o swap-selection-start-stop repaint-mode bind -s --preset -M visual f forward-jump @@ -306,7 +306,22 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' # Therefore it needs to be before setting fish_bind_mode. fish_vi_cursor set -g fish_cursor_selection_mode inclusive + function __fish_vi_key_bindings_on_mode_change --on-variable fish_bind_mode + switch $fish_bind_mode + case insert + set -g fish_cursor_end_mode exclusive + case '*' + set -g fish_cursor_end_mode inclusive + end + end + function __fish_vi_key_bindings_remove_handlers --on-variable __fish_active_key_bindings + functions --erase __fish_vi_key_bindings_remove_handlers + functions --erase __fish_vi_key_bindings_on_mode_change + functions --erase fish_vi_cursor_handle + functions --erase fish_vi_cursor_handle_preexec + set -e -g fish_cursor_end_mode + set -e -g fish_cursor_selection_mode + end set fish_bind_mode $init_mode - end diff --git a/src/editable_line.rs b/src/editable_line.rs index 27fa17a8d..d2e8ad134 100644 --- a/src/editable_line.rs +++ b/src/editable_line.rs @@ -123,6 +123,7 @@ impl EditableLine { self.position } pub fn set_position(&mut self, position: usize) { + assert!(position <= self.len()); self.position = position; } diff --git a/src/env_dispatch.rs b/src/env_dispatch.rs index eb4904470..7a38054d7 100644 --- a/src/env_dispatch.rs +++ b/src/env_dispatch.rs @@ -9,8 +9,8 @@ use crate::input_common::{update_wait_on_escape_ms, update_wait_on_sequence_key_ use crate::output::ColorSupport; use crate::proc::is_interactive_session; use crate::reader::{ - reader_change_cursor_selection_mode, reader_change_history, reader_schedule_prompt_repaint, - reader_set_autosuggestion_enabled, + reader_change_cursor_end_mode, reader_change_cursor_selection_mode, reader_change_history, + reader_schedule_prompt_repaint, reader_set_autosuggestion_enabled, }; use crate::screen::screen_set_midnight_commander_hack; use crate::screen::LAYOUT_CACHE_SHARED; @@ -85,6 +85,10 @@ static VAR_DISPATCH_TABLE: once_cell::sync::Lazy = L!("fish_cursor_selection_mode"), handle_fish_cursor_selection_mode_change, ); + table.add_anon( + L!("fish_cursor_end_mode"), + handle_fish_cursor_end_mode_change, + ); table }); @@ -259,6 +263,24 @@ fn handle_fish_cursor_selection_mode_change(vars: &EnvStack) { reader_change_cursor_selection_mode(mode); } +fn handle_fish_cursor_end_mode_change(vars: &EnvStack) { + use crate::reader::CursorEndMode; + + let inclusive = vars + .get(L!("fish_cursor_end_mode")) + .as_ref() + .map(|v| v.as_string()) + .map(|v| v == "inclusive") + .unwrap_or(false); + let mode = if inclusive { + CursorEndMode::Inclusive + } else { + CursorEndMode::Exclusive + }; + + reader_change_cursor_end_mode(mode); +} + fn handle_autosuggestion_change(vars: &EnvStack) { reader_set_autosuggestion_enabled(vars); } diff --git a/src/reader.rs b/src/reader.rs index 49dc05e03..f0ad63db2 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -338,6 +338,12 @@ pub enum CursorSelectionMode { Inclusive, } +#[derive(Eq, PartialEq)] +pub enum CursorEndMode { + Exclusive, + Inclusive, +} + /// A mode for calling the reader_kill function. enum Kill { /// In this mode, the new string is appended to the current contents of the kill buffer. @@ -500,6 +506,7 @@ pub struct ReaderData { /// The cursor selection mode. cursor_selection_mode: CursorSelectionMode, + cursor_end_mode: CursorEndMode, /// The selection data. If this is not none, then we have an active selection. selection: Option, @@ -814,7 +821,28 @@ pub fn reader_change_history(name: &wstr) { pub fn reader_change_cursor_selection_mode(selection_mode: CursorSelectionMode) { // We don't need to _change_ if we're not initialized yet. if let Some(data) = current_data() { + if data.cursor_selection_mode == selection_mode { + return; + } + let invalidates_selection = data.selection.is_some(); data.cursor_selection_mode = selection_mode; + if invalidates_selection { + data.update_buff_pos(EditableLineTag::Commandline, None); + } + } +} + +pub fn reader_change_cursor_end_mode(end_mode: CursorEndMode) { + // We don't need to _change_ if we're not initialized yet. + if let Some(data) = current_data() { + if data.cursor_end_mode == end_mode { + return; + } + let invalidates_end = data.selection.is_some(); + data.cursor_end_mode = end_mode; + if invalidates_end { + data.update_buff_pos(EditableLineTag::Commandline, None); + } } } @@ -1027,6 +1055,7 @@ impl ReaderData { history_pager_history_index_start: usize::MAX, history_pager_history_index_end: usize::MAX, cursor_selection_mode: CursorSelectionMode::Exclusive, + cursor_end_mode: CursorEndMode::Exclusive, selection: Default::default(), left_prompt_buff: Default::default(), mode_prompt_buff: Default::default(), @@ -1159,13 +1188,24 @@ impl ReaderData { } /// Update the cursor position. - fn update_buff_pos(&mut self, elt: EditableLineTag, new_pos: Option) { + fn update_buff_pos(&mut self, elt: EditableLineTag, mut new_pos: Option) -> bool { + if self.cursor_end_mode == CursorEndMode::Inclusive { + let el = self.edit_line(elt); + let mut pos = new_pos.unwrap_or(el.position()); + if !el.is_empty() && pos == el.len() { + pos = el.len() - 1; + if el.position() == pos { + return false; + } + new_pos = Some(pos); + } + } if let Some(pos) = new_pos { self.edit_line_mut(elt).set_position(pos); } if elt != EditableLineTag::Commandline { - return; + return true; } let buff_pos = self.command_line.position(); let target_char = if self.cursor_selection_mode == CursorSelectionMode::Inclusive { @@ -1174,7 +1214,7 @@ impl ReaderData { 0 }; let Some(selection) = self.selection.as_mut() else { - return; + return true; }; if selection.begin <= buff_pos { selection.start = selection.begin; @@ -1183,6 +1223,7 @@ impl ReaderData { selection.start = buff_pos; selection.stop = selection.begin + target_char; } + true } } @@ -2025,7 +2066,7 @@ impl ReaderData { } rl::EndOfLine => { let (_elt, el) = self.active_edit_line(); - if el.position() == el.len() { + if self.is_at_end(el) { self.accept_autosuggestion(true, false, MoveWordStyle::Punctuation); } else { loop { @@ -2040,7 +2081,9 @@ impl ReaderData { } position }; - self.update_buff_pos(self.active_edit_line_tag(), Some(position + 1)); + if !self.update_buff_pos(self.active_edit_line_tag(), Some(position + 1)) { + break; + } } } } @@ -2255,17 +2298,24 @@ impl ReaderData { let yank_str = kill_yank(); self.insert_string(self.active_edit_line_tag(), &yank_str); rls.yank_len = yank_str.len(); + if self.cursor_end_mode == CursorEndMode::Inclusive { + let (_elt, el) = self.active_edit_line(); + self.update_buff_pos(self.active_edit_line_tag(), Some(el.position() - 1)); + } } rl::YankPop => { if rls.yank_len != 0 { let (elt, el) = self.active_edit_line(); let yank_str = kill_yank_rotate(); let new_yank_len = yank_str.len(); - self.replace_substring( - elt, - el.position() - rls.yank_len..el.position(), - yank_str, - ); + let bias = if self.cursor_end_mode == CursorEndMode::Inclusive { + 1 + } else { + 0 + }; + let begin = el.position() + bias - rls.yank_len; + let end = el.position() + bias; + self.replace_substring(elt, begin..end, yank_str); self.update_buff_pos(elt, None); rls.yank_len = new_yank_len; self.suppress_autosuggestion = true; @@ -2448,14 +2498,14 @@ impl ReaderData { let (elt, el) = self.active_edit_line(); if self.is_navigating_pager_contents() { self.select_completion_in_direction(SelectionMotion::East, false); - } else if el.position() != el.len() { - self.update_buff_pos(elt, Some(el.position() + 1)); - } else { + } else if self.is_at_end(el) { self.accept_autosuggestion( /*full=*/ c != rl::ForwardSingleChar, /*single=*/ c == rl::ForwardSingleChar, MoveWordStyle::Punctuation, ); + } else { + self.update_buff_pos(elt, Some(el.position() + 1)); } } rl::BackwardKillWord | rl::BackwardKillPathComponent | rl::BackwardKillBigword => { @@ -2539,10 +2589,10 @@ impl ReaderData { MoveWordStyle::Whitespace }; let (elt, el) = self.active_edit_line(); - if el.position() != el.len() { - self.move_word(elt, MoveWordDir::Right, /*erase=*/ false, style, false); - } else { + if self.is_at_end(el) { self.accept_autosuggestion(false, false, style); + } else { + self.move_word(elt, MoveWordDir::Right, /*erase=*/ false, style, false); } } rl::BeginningOfHistory | rl::EndOfHistory => { @@ -2836,7 +2886,9 @@ impl ReaderData { if el.position() == 0 || el.text().as_char_slice()[el.position() - 1] == '\n' { break elt; } - self.update_buff_pos(elt, Some(el.position() - 1)); + if !self.update_buff_pos(elt, Some(el.position() - 1)) { + break elt; + } }; self.insert_char(elt, '\n'); let (elt, el) = self.active_edit_line(); @@ -2849,7 +2901,9 @@ impl ReaderData { { break elt; } - self.update_buff_pos(elt, Some(el.position() + 1)); + if !self.update_buff_pos(elt, Some(el.position() + 1)) { + break elt; + } }; self.insert_char(elt, '\n'); } @@ -3907,6 +3961,13 @@ impl ReaderData { debounce_autosuggestions().perform_with_completion(performer, completion); } + fn is_at_end(&self, el: &EditableLine) -> bool { + match self.cursor_end_mode { + CursorEndMode::Exclusive => el.position() == el.len(), + CursorEndMode::Inclusive => el.position() + 1 >= el.len(), + } + } + // Accept any autosuggestion by replacing the command line with it. If full is true, take the whole // thing; if it's false, then respect the passed in style. fn accept_autosuggestion(