vim: Handle exclusive-linewise edgecase correctly (#27786)

Before this change we didn't explicitly handle vim's exclusive-linewise
edgecase
(https://neovim.io/doc/user/motion.html#exclusive).

Instead we had hard-coded workarounds in a few places to make our tests
pass.
The most pernicious of these workarounds was that we represented a
visual line
selection as including the trailing newline (or leading newline for
files that
end with no newline), which other code had to undo to get back to what
the user
indended.

Closes #21440
Updates #6900

Release Notes:

- vim: Fixed `d]}` to not delete the closing brace
- vim: Fixed `d}` from the start of the line to not delete the paragraph
separator
- vim: Fixed `d}` from the middle of the line to not delete the final
newline
This commit is contained in:
Conrad Irwin 2025-03-31 10:36:20 -06:00 committed by GitHub
parent e1e8c1786e
commit fc269dfaf9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
27 changed files with 471 additions and 482 deletions

View file

@ -21,6 +21,26 @@ use crate::{
Vim,
};
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum MotionKind {
Linewise,
Exclusive,
Inclusive,
}
impl MotionKind {
pub(crate) fn for_mode(mode: Mode) -> Self {
match mode {
Mode::VisualLine => MotionKind::Linewise,
_ => MotionKind::Exclusive,
}
}
pub(crate) fn linewise(&self) -> bool {
matches!(self, MotionKind::Linewise)
}
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum Motion {
Left,
@ -622,7 +642,7 @@ impl Vim {
// Motion handling is specified here:
// https://github.com/vim/vim/blob/master/runtime/doc/motion.txt
impl Motion {
pub fn linewise(&self) -> bool {
fn default_kind(&self) -> MotionKind {
use Motion::*;
match self {
Down { .. }
@ -633,8 +653,6 @@ impl Motion {
| NextLineStart
| PreviousLineStart
| StartOfLineDownward
| StartOfParagraph
| EndOfParagraph
| WindowTop
| WindowMiddle
| WindowBottom
@ -649,37 +667,47 @@ impl Motion {
| NextComment
| PreviousComment
| GoToPercentage
| Jump { line: true, .. } => true,
| Jump { line: true, .. } => MotionKind::Linewise,
EndOfLine { .. }
| EndOfLineDownward
| Matching
| UnmatchedForward { .. }
| UnmatchedBackward { .. }
| FindForward { .. }
| Left
| NextWordEnd { .. }
| PreviousWordEnd { .. }
| NextSubwordEnd { .. }
| PreviousSubwordEnd { .. } => MotionKind::Inclusive,
Left
| WrappingLeft
| Right
| SentenceBackward
| SentenceForward
| WrappingRight
| StartOfLine { .. }
| EndOfLineDownward
| StartOfParagraph
| EndOfParagraph
| SentenceBackward
| SentenceForward
| GoToColumn
| UnmatchedForward { .. }
| UnmatchedBackward { .. }
| NextWordStart { .. }
| NextWordEnd { .. }
| PreviousWordStart { .. }
| PreviousWordEnd { .. }
| NextSubwordStart { .. }
| NextSubwordEnd { .. }
| PreviousSubwordStart { .. }
| PreviousSubwordEnd { .. }
| FirstNonWhitespace { .. }
| FindBackward { .. }
| Sneak { .. }
| SneakBackward { .. }
| RepeatFind { .. }
| RepeatFindReversed { .. }
| Jump { line: false, .. }
| ZedSearchResult { .. } => false,
| Jump { .. }
| ZedSearchResult { .. } => MotionKind::Exclusive,
RepeatFind { last_find: motion } | RepeatFindReversed { last_find: motion } => {
motion.default_kind()
}
}
}
fn skip_exclusive_special_case(&self) -> bool {
match self {
Motion::WrappingLeft | Motion::WrappingRight => true,
_ => false,
}
}
@ -741,67 +769,6 @@ impl Motion {
}
}
pub fn inclusive(&self) -> bool {
use Motion::*;
match self {
Down { .. }
| Up { .. }
| StartOfDocument
| EndOfDocument
| CurrentLine
| EndOfLine { .. }
| EndOfLineDownward
| Matching
| GoToPercentage
| UnmatchedForward { .. }
| UnmatchedBackward { .. }
| FindForward { .. }
| WindowTop
| WindowMiddle
| WindowBottom
| NextWordEnd { .. }
| PreviousWordEnd { .. }
| NextSubwordEnd { .. }
| PreviousSubwordEnd { .. }
| NextLineStart
| PreviousLineStart => true,
Left
| WrappingLeft
| Right
| WrappingRight
| StartOfLine { .. }
| StartOfLineDownward
| StartOfParagraph
| EndOfParagraph
| SentenceBackward
| SentenceForward
| GoToColumn
| NextWordStart { .. }
| PreviousWordStart { .. }
| NextSubwordStart { .. }
| PreviousSubwordStart { .. }
| FirstNonWhitespace { .. }
| FindBackward { .. }
| Sneak { .. }
| SneakBackward { .. }
| Jump { .. }
| NextSectionStart
| NextSectionEnd
| PreviousSectionStart
| PreviousSectionEnd
| NextMethodStart
| NextMethodEnd
| PreviousMethodStart
| PreviousMethodEnd
| NextComment
| PreviousComment
| ZedSearchResult { .. } => false,
RepeatFind { last_find: motion } | RepeatFindReversed { last_find: motion } => {
motion.inclusive()
}
}
}
pub fn move_point(
&self,
map: &DisplaySnapshot,
@ -1153,9 +1120,8 @@ impl Motion {
map: &DisplaySnapshot,
selection: Selection<DisplayPoint>,
times: Option<usize>,
expand_to_surrounding_newline: bool,
text_layout_details: &TextLayoutDetails,
) -> Option<Range<DisplayPoint>> {
) -> Option<(Range<DisplayPoint>, MotionKind)> {
if let Motion::ZedSearchResult {
prior_selections,
new_selections,
@ -1174,89 +1140,88 @@ impl Motion {
.max(prior_selection.end.to_display_point(map));
if start < end {
return Some(start..end);
return Some((start..end, MotionKind::Exclusive));
} else {
return Some(end..start);
return Some((end..start, MotionKind::Exclusive));
}
} else {
return None;
}
}
if let Some((new_head, goal)) = self.move_point(
let (new_head, goal) = self.move_point(
map,
selection.head(),
selection.goal,
times,
text_layout_details,
) {
let mut selection = selection.clone();
selection.set_head(new_head, goal);
)?;
let mut selection = selection.clone();
selection.set_head(new_head, goal);
if self.linewise() {
selection.start = map.prev_line_boundary(selection.start.to_point(map)).1;
let mut kind = self.default_kind();
if expand_to_surrounding_newline {
if selection.end.row() < map.max_point().row() {
*selection.end.row_mut() += 1;
*selection.end.column_mut() = 0;
selection.end = map.clip_point(selection.end, Bias::Right);
// Don't reset the end here
return Some(selection.start..selection.end);
} else if selection.start.row().0 > 0 {
*selection.start.row_mut() -= 1;
*selection.start.column_mut() = map.line_len(selection.start.row());
selection.start = map.clip_point(selection.start, Bias::Left);
}
if let Motion::NextWordStart {
ignore_punctuation: _,
} = self
{
// Another special case: When using the "w" motion in combination with an
// operator and the last word moved over is at the end of a line, the end of
// that word becomes the end of the operated text, not the first word in the
// next line.
let start = selection.start.to_point(map);
let end = selection.end.to_point(map);
let start_row = MultiBufferRow(selection.start.to_point(map).row);
if end.row > start.row {
selection.end = Point::new(start_row.0, map.buffer_snapshot.line_len(start_row))
.to_display_point(map);
// a bit of a hack, we need `cw` on a blank line to not delete the newline,
// but dw on a blank line should. The `Linewise` returned from this method
// causes the `d` operator to include the trailing newline.
if selection.start == selection.end {
return Some((selection.start..selection.end, MotionKind::Linewise));
}
}
} else if kind == MotionKind::Exclusive && !self.skip_exclusive_special_case() {
let start_point = selection.start.to_point(map);
let mut end_point = selection.end.to_point(map);
selection.end = map.next_line_boundary(selection.end.to_point(map)).1;
} else {
// Another special case: When using the "w" motion in combination with an
// operator and the last word moved over is at the end of a line, the end of
// that word becomes the end of the operated text, not the first word in the
// next line.
if let Motion::NextWordStart {
ignore_punctuation: _,
} = self
{
let start_row = MultiBufferRow(selection.start.to_point(map).row);
if selection.end.to_point(map).row > start_row.0 {
selection.end =
Point::new(start_row.0, map.buffer_snapshot.line_len(start_row))
.to_display_point(map)
if end_point.row > start_point.row {
let first_non_blank_of_start_row = map
.line_indent_for_buffer_row(MultiBufferRow(start_point.row))
.raw_len();
// https://github.com/neovim/neovim/blob/ee143aaf65a0e662c42c636aa4a959682858b3e7/src/nvim/ops.c#L6178-L6203
if end_point.column == 0 {
// If the motion is exclusive and the end of the motion is in column 1, the
// end of the motion is moved to the end of the previous line and the motion
// becomes inclusive. Example: "}" moves to the first line after a paragraph,
// but "d}" will not include that line.
//
// If the motion is exclusive, the end of the motion is in column 1 and the
// start of the motion was at or before the first non-blank in the line, the
// motion becomes linewise. Example: If a paragraph begins with some blanks
// and you do "d}" while standing on the first non-blank, all the lines of
// the paragraph are deleted, including the blanks.
if start_point.column <= first_non_blank_of_start_row {
kind = MotionKind::Linewise;
} else {
kind = MotionKind::Inclusive;
}
}
// If the motion is exclusive and the end of the motion is in column 1, the
// end of the motion is moved to the end of the previous line and the motion
// becomes inclusive. Example: "}" moves to the first line after a paragraph,
// but "d}" will not include that line.
let mut inclusive = self.inclusive();
let start_point = selection.start.to_point(map);
let mut end_point = selection.end.to_point(map);
// DisplayPoint
if !inclusive
&& self != &Motion::WrappingLeft
&& end_point.row > start_point.row
&& end_point.column == 0
{
inclusive = true;
end_point.row -= 1;
end_point.column = 0;
selection.end = map.clip_point(map.next_line_boundary(end_point).1, Bias::Left);
}
if inclusive && selection.end.column() < map.line_len(selection.end.row()) {
selection.end = movement::saturating_right(map, selection.end)
}
}
Some(selection.start..selection.end)
} else {
None
} else if kind == MotionKind::Inclusive {
selection.end = movement::saturating_right(map, selection.end)
}
if kind == MotionKind::Linewise {
selection.start = map.prev_line_boundary(selection.start.to_point(map)).1;
selection.end = map.next_line_boundary(selection.end.to_point(map)).1;
}
Some((selection.start..selection.end, kind))
}
// Expands a selection using self for an operator
@ -1265,22 +1230,12 @@ impl Motion {
map: &DisplaySnapshot,
selection: &mut Selection<DisplayPoint>,
times: Option<usize>,
expand_to_surrounding_newline: bool,
text_layout_details: &TextLayoutDetails,
) -> bool {
if let Some(range) = self.range(
map,
selection.clone(),
times,
expand_to_surrounding_newline,
text_layout_details,
) {
selection.start = range.start;
selection.end = range.end;
true
} else {
false
}
) -> Option<MotionKind> {
let (range, kind) = self.range(map, selection.clone(), times, text_layout_details)?;
selection.start = range.start;
selection.end = range.end;
Some(kind)
}
}