From 759646e0a35a2c4586817b79028cb347e3749de4 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Fri, 20 Sep 2024 11:45:03 -0400 Subject: [PATCH] editor: Improve rewrapping when working with comments at different indentation levels (#18146) This PR improves the `editor::Rewrap` command when working with comments that were not all at the same indentation level. We now use a heuristic of finding the most common indentation level for each line, using the deepest indent in the event of a tie. It also removes an `.unwrap()` that would previously lead to a panic in this case. Instead of unwrapping we now log an error to the logs and skip rewrapping for that selection. Release Notes: - Improved the behavior of `editor: rewrap` when working with a selection that contained comments at different indentation levels. --- crates/editor/src/editor.rs | 46 ++++++++++++++++--- crates/editor/src/editor_tests.rs | 74 +++++++++++++++++++++++++++++++ crates/language/src/buffer.rs | 4 +- 3 files changed, 116 insertions(+), 8 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index eb2dafc24dc0f33d0052bc244dacfd0625aa3531..33eb51cb0ecb777f3f993b93e341ec3139a8e95a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6736,9 +6736,31 @@ impl Editor { } } - let row = selection.head().row; - let indent_size = buffer.indent_size_for_line(MultiBufferRow(row)); - let indent_end = Point::new(row, indent_size.len); + // Since not all lines in the selection may be at the same indent + // level, choose the indent size that is the most common between all + // of the lines. + // + // If there is a tie, we use the deepest indent. + let (indent_size, indent_end) = { + let mut indent_size_occurrences = HashMap::default(); + let mut rows_by_indent_size = HashMap::>::default(); + + for row in start_row..=end_row { + let indent = buffer.indent_size_for_line(MultiBufferRow(row)); + rows_by_indent_size.entry(indent).or_default().push(row); + *indent_size_occurrences.entry(indent).or_insert(0) += 1; + } + + let indent_size = indent_size_occurrences + .into_iter() + .max_by_key(|(indent, count)| (*count, indent.len)) + .map(|(indent, _)| indent) + .unwrap_or_default(); + let row = rows_by_indent_size[&indent_size][0]; + let indent_end = Point::new(row, indent_size.len); + + (indent_size, indent_end) + }; let mut line_prefix = indent_size.chars().collect::(); @@ -6788,10 +6810,22 @@ impl Editor { let start = Point::new(start_row, 0); let end = Point::new(end_row, buffer.line_len(MultiBufferRow(end_row))); let selection_text = buffer.text_for_range(start..end).collect::(); - let unwrapped_text = selection_text + let Some(lines_without_prefixes) = selection_text .lines() - .map(|line| line.strip_prefix(&line_prefix).unwrap()) - .join(" "); + .map(|line| { + line.strip_prefix(&line_prefix) + .or_else(|| line.trim_start().strip_prefix(&line_prefix.trim_start())) + .ok_or_else(|| { + anyhow!("line did not start with prefix {line_prefix:?}: {line:?}") + }) + }) + .collect::, _>>() + .log_err() + else { + continue; + }; + + let unwrapped_text = lines_without_prefixes.join(" "); let wrap_column = buffer .settings_at(Point::new(start_row, 0), cx) .preferred_line_length as usize; diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 589673447d7f48819db5f9bf229282ddf6b5ab69..85684db8181333e17bd69ca920d6084ef5fdc000 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -4249,6 +4249,80 @@ async fn test_rewrap(cx: &mut TestAppContext) { cx.update_editor(|e, cx| e.rewrap(&Rewrap, cx)); cx.assert_editor_state(wrapped_text); } + + // Test rewrapping unaligned comments in a selection. + { + let language = Arc::new(Language::new( + LanguageConfig { + line_comments: vec!["// ".into(), "/// ".into()], + ..LanguageConfig::default() + }, + Some(tree_sitter_rust::LANGUAGE.into()), + )); + cx.update_buffer(|buffer, cx| buffer.set_language(Some(language), cx)); + + let unwrapped_text = indoc! {" + fn foo() { + if true { + « // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit purus, a ornare lacus gravida vitae. + // Praesent semper egestas tellus id dignissim.ˇ» + do_something(); + } else { + // + } + + } + "}; + + let wrapped_text = indoc! {" + fn foo() { + if true { + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus + // mollis elit purus, a ornare lacus gravida vitae. Praesent semper + // egestas tellus id dignissim.ˇ + do_something(); + } else { + // + } + + } + "}; + + cx.set_state(unwrapped_text); + cx.update_editor(|e, cx| e.rewrap(&Rewrap, cx)); + cx.assert_editor_state(wrapped_text); + + let unwrapped_text = indoc! {" + fn foo() { + if true { + «ˇ // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus mollis elit purus, a ornare lacus gravida vitae. + // Praesent semper egestas tellus id dignissim.» + do_something(); + } else { + // + } + + } + "}; + + let wrapped_text = indoc! {" + fn foo() { + if true { + // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus + // mollis elit purus, a ornare lacus gravida vitae. Praesent semper + // egestas tellus id dignissim.ˇ + do_something(); + } else { + // + } + + } + "}; + + cx.set_state(unwrapped_text); + cx.update_editor(|e, cx| e.rewrap(&Rewrap, cx)); + cx.assert_editor_state(wrapped_text); + } } #[gpui::test] diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 08fc1ccdb45d5b0f47420a05cedfadbc0c5d66a9..acb57273e30eede980729ca2e4658373b3fe7d8a 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -144,7 +144,7 @@ pub struct BufferSnapshot { /// The kind and amount of indentation in a particular line. For now, /// assumes that indentation is all the same character. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Default)] pub struct IndentSize { /// The number of bytes that comprise the indentation. pub len: u32, @@ -153,7 +153,7 @@ pub struct IndentSize { } /// A whitespace character that's used for indentation. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Default)] pub enum IndentKind { /// An ASCII space character. #[default]