Skip to content

Commit

Permalink
Use char level diff for Rewrap action for cursor preservation + not r…
Browse files Browse the repository at this point in the history
…einserting all text (#20368)

Closes #18896

Release Notes:

- Fixed #18896 - `editor::Rewrap` now preserves cursors and only inserts
whitespace by using character-level diff instead of line-level diff.
  • Loading branch information
mgsloan authored Nov 12, 2024
1 parent 6819108 commit ab20681
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 31 deletions.
19 changes: 15 additions & 4 deletions crates/editor/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,12 @@ pub trait Addon: 'static {
fn to_any(&self) -> &dyn std::any::Any;
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum IsVimMode {
Yes,
No,
}

/// 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 @@ -7022,10 +7028,10 @@ impl Editor {
}

pub fn rewrap(&mut self, _: &Rewrap, cx: &mut ViewContext<Self>) {
self.rewrap_impl(true, cx)
self.rewrap_impl(IsVimMode::No, cx)
}

pub fn rewrap_impl(&mut self, only_text: bool, cx: &mut ViewContext<Self>) {
pub fn rewrap_impl(&mut self, is_vim_mode: IsVimMode, cx: &mut ViewContext<Self>) {
let buffer = self.buffer.read(cx).snapshot(cx);
let selections = self.selections.all::<Point>(cx);
let mut selections = selections.iter().peekable();
Expand All @@ -7046,7 +7052,7 @@ impl Editor {
continue;
}

let mut should_rewrap = !only_text;
let mut should_rewrap = is_vim_mode == IsVimMode::Yes;

if let Some(language_scope) = buffer.language_scope_at(selection.head()) {
match language_scope.language_name().0.as_ref() {
Expand Down Expand Up @@ -7158,7 +7164,12 @@ impl Editor {
tab_size,
);

let diff = TextDiff::from_lines(&selection_text, &wrapped_text);
// TODO: should always use char-based diff while still supporting cursor behavior that
// matches vim.
let diff = match is_vim_mode {
IsVimMode::Yes => TextDiff::from_lines(&selection_text, &wrapped_text),
IsVimMode::No => TextDiff::from_chars(&selection_text, &wrapped_text),
};
let mut offset = start.to_offset(&buffer);
let mut moved_since_edit = true;

Expand Down
46 changes: 23 additions & 23 deletions crates/editor/src/editor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4205,7 +4205,7 @@ async fn test_rewrap(cx: &mut TestAppContext) {
// ˇLorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus auctor, eu lacinia sapien scelerisque. Vivamus sit amet neque et quam tincidunt hendrerit. Praesent semper egestas tellus id dignissim. Pellentesque odio lectus, iaculis ac volutpat et, blandit quis urna. Sed vestibulum nisi sit amet nisl venenatis tempus. Donec molestie blandit quam, et porta nunc laoreet in. Integer sit amet scelerisque nisi. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras egestas porta metus, eu viverra ipsum efficitur quis. Donec luctus eros turpis, id vulputate turpis porttitor id. Aliquam id accumsan eros.
"},
indoc! {"
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
// ˇLorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
// purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus
// auctor, eu lacinia sapien scelerisque. Vivamus sit amet neque et quam
// tincidunt hendrerit. Praesent semper egestas tellus id dignissim.
Expand All @@ -4214,7 +4214,7 @@ async fn test_rewrap(cx: &mut TestAppContext) {
// et porta nunc laoreet in. Integer sit amet scelerisque nisi. Lorem ipsum
// dolor sit amet, consectetur adipiscing elit. Cras egestas porta metus, eu
// viverra ipsum efficitur quis. Donec luctus eros turpis, id vulputate turpis
// porttitor id. Aliquam id accumsan eros.ˇ
// porttitor id. Aliquam id accumsan eros.
"},
language_with_c_comments.clone(),
&mut cx,
Expand All @@ -4226,7 +4226,7 @@ async fn test_rewrap(cx: &mut TestAppContext) {
«// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus auctor, eu lacinia sapien scelerisque. Vivamus sit amet neque et quam tincidunt hendrerit. Praesent semper egestas tellus id dignissim. Pellentesque odio lectus, iaculis ac volutpat et, blandit quis urna. Sed vestibulum nisi sit amet nisl venenatis tempus. Donec molestie blandit quam, et porta nunc laoreet in. Integer sit amet scelerisque nisi. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras egestas porta metus, eu viverra ipsum efficitur quis. Donec luctus eros turpis, id vulputate turpis porttitor id. Aliquam id accumsan eros.ˇ»
"},
indoc! {"
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
«// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
// purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus
// auctor, eu lacinia sapien scelerisque. Vivamus sit amet neque et quam
// tincidunt hendrerit. Praesent semper egestas tellus id dignissim.
Expand All @@ -4235,7 +4235,7 @@ async fn test_rewrap(cx: &mut TestAppContext) {
// et porta nunc laoreet in. Integer sit amet scelerisque nisi. Lorem ipsum
// dolor sit amet, consectetur adipiscing elit. Cras egestas porta metus, eu
// viverra ipsum efficitur quis. Donec luctus eros turpis, id vulputate turpis
// porttitor id. Aliquam id accumsan eros.ˇ
// porttitor id. Aliquam id accumsan eros.ˇ»
"},
language_with_c_comments.clone(),
&mut cx,
Expand All @@ -4250,16 +4250,16 @@ async fn test_rewrap(cx: &mut TestAppContext) {
// ˇblandit quis urna. Sed vestibulum nisi sit amet nisl venenatis tempus. Donec molestie blandit quam, et porta nunc laoreet in. Integer sit amet scelerisque nisi. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras egestas porta metus, eu viverra ipsum efficitur quis. Donec luctus eros turpis, id vulputate turpis porttitor id. Aliquam id accumsan eros.
"},
indoc! {"
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
// ˇLorem ipsum dolor sit amet, consectetur adipiscing elit. ˇVivamus mollis elit
// purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus
// auctor, eu lacinia sapien scelerisque. Vivamus sit amet neque et quam
// auctor, eu lacinia sapien scelerisque. ˇVivamus sit amet neque et quam
// tincidunt hendrerit. Praesent semper egestas tellus id dignissim.
// Pellentesque odio lectus, iaculis ac volutpat et, blandit quis urna. Sed
// Pellentesque odio lectus, iaculis ac volutpat et, ˇblandit quis urna. Sed
// vestibulum nisi sit amet nisl venenatis tempus. Donec molestie blandit quam,
// et porta nunc laoreet in. Integer sit amet scelerisque nisi. Lorem ipsum
// dolor sit amet, consectetur adipiscing elit. Cras egestas porta metus, eu
// viverra ipsum efficitur quis. Donec luctus eros turpis, id vulputate turpis
// porttitor id. Aliquam id accumsan eros.ˇ
// porttitor id. Aliquam id accumsan eros.
"},
language_with_c_comments.clone(),
&mut cx,
Expand All @@ -4275,17 +4275,17 @@ async fn test_rewrap(cx: &mut TestAppContext) {
// ˇblandit quis urna. Sed vestibulum nisi sit amet nisl venenatis tempus. Donec molestie blandit quam, et porta nunc laoreet in. Integer sit amet scelerisque nisi. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras egestas porta metus, eu viverra ipsum efficitur quis. Donec luctus eros turpis, id vulputate turpis porttitor id. Aliquam id accumsan eros.
"},
indoc! {"
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
// ˇLorem ipsum dolor sit amet, consectetur adipiscing elit. ˇVivamus mollis elit
// purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus
// auctor, eu lacinia sapien scelerisque.ˇ
// auctor, eu lacinia sapien scelerisque.
//
// Vivamus sit amet neque et quam tincidunt hendrerit. Praesent semper egestas
// ˇVivamus sit amet neque et quam tincidunt hendrerit. Praesent semper egestas
// tellus id dignissim. Pellentesque odio lectus, iaculis ac volutpat et,
// blandit quis urna. Sed vestibulum nisi sit amet nisl venenatis tempus. Donec
// ˇblandit quis urna. Sed vestibulum nisi sit amet nisl venenatis tempus. Donec
// molestie blandit quam, et porta nunc laoreet in. Integer sit amet scelerisque
// nisi. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras egestas
// porta metus, eu viverra ipsum efficitur quis. Donec luctus eros turpis, id
// vulputate turpis porttitor id. Aliquam id accumsan eros.ˇ
// vulputate turpis porttitor id. Aliquam id accumsan eros.
"},
language_with_c_comments.clone(),
&mut cx,
Expand All @@ -4297,7 +4297,7 @@ async fn test_rewrap(cx: &mut TestAppContext) {
# ˇLorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus auctor, eu lacinia sapien scelerisque. Vivamus sit amet neque et quam tincidunt hendrerit. Praesent semper egestas tellus id dignissim. Pellentesque odio lectus, iaculis ac volutpat et, blandit quis urna. Sed vestibulum nisi sit amet nisl venenatis tempus. Donec molestie blandit quam, et porta nunc laoreet in. Integer sit amet scelerisque nisi. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras egestas porta metus, eu viverra ipsum efficitur quis. Donec luctus eros turpis, id vulputate turpis porttitor id. Aliquam id accumsan eros.
"},
indoc! {"
# Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
# ˇLorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
# purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus auctor,
# eu lacinia sapien scelerisque. Vivamus sit amet neque et quam tincidunt
# hendrerit. Praesent semper egestas tellus id dignissim. Pellentesque odio
Expand All @@ -4306,7 +4306,7 @@ async fn test_rewrap(cx: &mut TestAppContext) {
# in. Integer sit amet scelerisque nisi. Lorem ipsum dolor sit amet, consectetur
# adipiscing elit. Cras egestas porta metus, eu viverra ipsum efficitur quis.
# Donec luctus eros turpis, id vulputate turpis porttitor id. Aliquam id
# accumsan eros.ˇ
# accumsan eros.
"},
language_with_pound_comments.clone(),
&mut cx,
Expand Down Expand Up @@ -4342,13 +4342,13 @@ async fn test_rewrap(cx: &mut TestAppContext) {
indoc! {"
# Hello
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
Lorem ipsum dolor sit amet, ˇconsectetur adipiscing elit. Vivamus mollis elit
purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus auctor,
eu lacinia sapien scelerisque. Vivamus sit amet neque et quam tincidunt
hendrerit. Praesent semper egestas tellus id dignissim. Pellentesque odio
lectus, iaculis ac volutpat et, blandit quis urna. Sed vestibulum nisi sit amet
nisl venenatis tempus. Donec molestie blandit quam, et porta nunc laoreet in.
Integer sit amet scelerisque nisi.ˇ
Integer sit amet scelerisque nisi.
"},
markdown_language,
&mut cx,
Expand All @@ -4359,13 +4359,13 @@ async fn test_rewrap(cx: &mut TestAppContext) {
Lorem ipsum dolor sit amet, ˇconsectetur adipiscing elit. Vivamus mollis elit purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus auctor, eu lacinia sapien scelerisque. Vivamus sit amet neque et quam tincidunt hendrerit. Praesent semper egestas tellus id dignissim. Pellentesque odio lectus, iaculis ac volutpat et, blandit quis urna. Sed vestibulum nisi sit amet nisl venenatis tempus. Donec molestie blandit quam, et porta nunc laoreet in. Integer sit amet scelerisque nisi.
"},
indoc! {"
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit
Lorem ipsum dolor sit amet, ˇconsectetur adipiscing elit. Vivamus mollis elit
purus, a ornare lacus gravida vitae. Proin consectetur felis vel purus auctor,
eu lacinia sapien scelerisque. Vivamus sit amet neque et quam tincidunt
hendrerit. Praesent semper egestas tellus id dignissim. Pellentesque odio
lectus, iaculis ac volutpat et, blandit quis urna. Sed vestibulum nisi sit amet
nisl venenatis tempus. Donec molestie blandit quam, et porta nunc laoreet in.
Integer sit amet scelerisque nisi.ˇ
Integer sit amet scelerisque nisi.
"},
plaintext_language,
&mut cx,
Expand All @@ -4387,9 +4387,9 @@ async fn test_rewrap(cx: &mut TestAppContext) {
indoc! {"
fn foo() {
if true {
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus
« // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus
// mollis elit purus, a ornare lacus gravida vitae. Praesent semper
// egestas tellus id dignissim.ˇ
// egestas tellus id dignissim.ˇ»
do_something();
} else {
//
Expand All @@ -4416,9 +4416,9 @@ async fn test_rewrap(cx: &mut TestAppContext) {
indoc! {"
fn foo() {
if true {
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus
«ˇ // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus
// mollis elit purus, a ornare lacus gravida vitae. Praesent semper
// egestas tellus id dignissim.ˇ
// egestas tellus id dignissim.»
do_something();
} else {
//
Expand Down
8 changes: 4 additions & 4 deletions crates/vim/src/rewrap.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{motion::Motion, object::Object, state::Mode, Vim};
use collections::HashMap;
use editor::{display_map::ToDisplayPoint, scroll::Autoscroll, Bias, Editor};
use editor::{display_map::ToDisplayPoint, scroll::Autoscroll, Bias, Editor, IsVimMode};
use gpui::actions;
use language::SelectionGoal;
use ui::ViewContext;
Expand All @@ -15,7 +15,7 @@ pub(crate) fn register(editor: &mut Editor, cx: &mut ViewContext<Vim>) {
vim.update_editor(cx, |vim, editor, cx| {
editor.transact(cx, |editor, cx| {
let mut positions = vim.save_selection_starts(editor, cx);
editor.rewrap_impl(false, cx);
editor.rewrap_impl(IsVimMode::Yes, cx);
editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
s.move_with(|map, selection| {
if let Some(anchor) = positions.remove(&selection.id) {
Expand Down Expand Up @@ -52,7 +52,7 @@ impl Vim {
motion.expand_selection(map, selection, times, false, &text_layout_details);
});
});
editor.rewrap_impl(false, cx);
editor.rewrap_impl(IsVimMode::Yes, cx);
editor.change_selections(None, cx, |s| {
s.move_with(|map, selection| {
let anchor = selection_starts.remove(&selection.id).unwrap();
Expand Down Expand Up @@ -82,7 +82,7 @@ impl Vim {
object.expand_selection(map, selection, around);
});
});
editor.rewrap_impl(false, cx);
editor.rewrap_impl(IsVimMode::Yes, cx);
editor.change_selections(None, cx, |s| {
s.move_with(|map, selection| {
let anchor = original_positions.remove(&selection.id).unwrap();
Expand Down

0 comments on commit ab20681

Please sign in to comment.