From 803cf8b55074bad80613a81f92875858757c3190 Mon Sep 17 00:00:00 2001 From: Rose Hogenson Date: Tue, 7 Jan 2025 20:34:12 -0800 Subject: [PATCH 1/6] Reuse soft-wrap infrastructure in :reflow. I changed :reflow to use the DocumentFormatter object instead of the textwrap crate. This allows using the same logic for soft wrap as for :reflow. Because the logic is the same as for soft wrap, we end up preserving all existing newlines, so it's more like "wrap" than reflow, but I think this behavior makes sense anyway to avoid extraneous diffs. --- Cargo.lock | 32 +----------- helix-core/Cargo.toml | 2 - helix-core/src/doc_formatter.rs | 61 ++++++++++++++++++++--- helix-core/src/doc_formatter/test.rs | 2 +- helix-core/src/lib.rs | 1 - helix-core/src/wrap.rs | 11 ---- helix-term/src/commands/typed.rs | 31 +++++++++--- helix-term/tests/test/commands.rs | 36 +++++++++++++ helix-view/src/annotations/diagnostics.rs | 2 +- helix-view/src/document.rs | 2 +- 10 files changed, 118 insertions(+), 62 deletions(-) delete mode 100644 helix-core/src/wrap.rs diff --git a/Cargo.lock b/Cargo.lock index cabb98a70..9ac28b0b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1345,12 +1345,11 @@ dependencies = [ "slotmap", "smallvec", "smartstring", - "textwrap", "toml", "tree-sitter", "unicode-general-category", "unicode-segmentation", - "unicode-width 0.1.12", + "unicode-width", "url", ] @@ -2427,12 +2426,6 @@ dependencies = [ "version_check", ] -[[package]] -name = "smawk" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" - [[package]] name = "socket2" version = "0.5.7" @@ -2505,17 +2498,6 @@ dependencies = [ "home", ] -[[package]] -name = "textwrap" -version = "0.16.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c13547615a44dc9c452a8a534638acdf07120d4b6847c8178705da06306a3057" -dependencies = [ - "smawk", - "unicode-linebreak", - "unicode-width 0.2.0", -] - [[package]] name = "thiserror" version = "1.0.69" @@ -2701,12 +2683,6 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" -[[package]] -name = "unicode-linebreak" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" - [[package]] name = "unicode-normalization" version = "0.1.23" @@ -2728,12 +2704,6 @@ version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68f5e5f3158ecfd4b8ff6fe086db7c8467a2dfdac97fe420f2b7c4aa97af66d6" -[[package]] -name = "unicode-width" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fc81956842c57dac11422a97c3b8195a1ff727f06e85c84ed2e8aa277c9a0fd" - [[package]] name = "url" version = "2.5.4" diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml index 10fb5a52c..e3fece59c 100644 --- a/helix-core/Cargo.toml +++ b/helix-core/Cargo.toml @@ -53,8 +53,6 @@ encoding_rs = "0.8" chrono = { version = "0.4", default-features = false, features = ["alloc", "std"] } -textwrap = "0.16.2" - nucleo.workspace = true parking_lot.workspace = true globset = "0.4.16" diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index d74709420..6c426354d 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -24,7 +24,7 @@ use helix_stdx::rope::{RopeGraphemes, RopeSliceExt}; use crate::graphemes::{Grapheme, GraphemeStr}; use crate::syntax::Highlight; use crate::text_annotations::TextAnnotations; -use crate::{Position, RopeSlice}; +use crate::{movement, Change, LineEnding, Position, Rope, RopeSlice, Tendril}; /// TODO make Highlight a u32 to reduce the size of this enum to a single word. #[derive(Debug, Clone, Copy)] @@ -146,7 +146,7 @@ impl<'a> GraphemeWithSource<'a> { pub struct TextFormat { pub soft_wrap: bool, pub tab_width: u16, - pub max_wrap: u16, + pub max_wrap: Option, pub max_indent_retain: u16, pub wrap_indicator: Box, pub wrap_indicator_highlight: Option, @@ -160,7 +160,7 @@ impl Default for TextFormat { TextFormat { soft_wrap: false, tab_width: 4, - max_wrap: 3, + max_wrap: Some(3), max_indent_retain: 4, wrap_indicator: Box::from(" "), viewport_width: 17, @@ -386,16 +386,22 @@ impl<'t> DocumentFormatter<'t> { .peek_grapheme(col, char_pos) .is_some_and(|grapheme| grapheme.is_newline() || grapheme.is_eof()) => { } - Ordering::Equal if word_width > self.text_fmt.max_wrap as usize => return, - Ordering::Greater if word_width > self.text_fmt.max_wrap as usize => { + Ordering::Equal + if word_width > self.text_fmt.max_wrap.map_or(usize::MAX, usize::from) => + { + return; + } + Ordering::Greater + if word_width > self.text_fmt.max_wrap.map_or(usize::MAX, usize::from) => + { self.peeked_grapheme = self.word_buf.pop(); return; } - Ordering::Equal | Ordering::Greater => { + Ordering::Equal | Ordering::Greater if self.visual_pos.col > 0 => { word_width = self.wrap_word(); col = self.visual_pos.col + word_width; } - Ordering::Less => (), + _ => (), } let Some(grapheme) = self.next_grapheme(col, char_pos) else { @@ -428,6 +434,47 @@ impl<'t> DocumentFormatter<'t> { pub fn next_visual_pos(&self) -> Position { self.visual_pos } + + fn find_indent<'a>(&self, line: usize, doc: RopeSlice<'a>) -> RopeSlice<'a> { + let line_start = doc.line_to_char(line); + let indent_end = movement::skip_while(doc, line_start, |ch| matches!(ch, ' ' | '\t')) + .unwrap_or(line_start); + let indent_end = movement::skip_while(doc, indent_end, |ch| matches!(ch, ' ' | '\t')) + .unwrap_or(indent_end); + return doc.slice(line_start..indent_end); + } + + /// consumes the iterator and hard-wraps the input where soft wraps would + /// have been applied. It probably only makes sense to call this method if + /// soft_wrap is true. + pub fn reflow(&mut self, doc: &Rope, line_ending: LineEnding) -> Vec { + let slice = doc.slice(..); + let mut current_line = self.visual_pos.row; + let mut changes = Vec::new(); + while let Some(grapheme) = self.next() { + if !grapheme.is_whitespace() && grapheme.visual_pos.row != current_line { + let indent = Tendril::from(format!( + "{}{}", + line_ending.as_str(), + self.find_indent(doc.char_to_line(grapheme.char_idx - 1), slice) + )); + let mut whitespace_start = grapheme.char_idx; + let mut whitespace_end = grapheme.char_idx; + while whitespace_start > 0 && slice.char(whitespace_start - 1) == ' ' { + whitespace_start -= 1; + } + while whitespace_end < slice.chars().len() && slice.char(whitespace_end) == ' ' { + whitespace_end += 1; + } + changes.push((whitespace_start, whitespace_end, Some(indent))); + current_line = grapheme.visual_pos.row; + } + if grapheme.raw == Grapheme::Newline { + current_line += 1; + } + } + changes + } } impl<'t> Iterator for DocumentFormatter<'t> { diff --git a/helix-core/src/doc_formatter/test.rs b/helix-core/src/doc_formatter/test.rs index 21be2e535..6cee5d3fb 100644 --- a/helix-core/src/doc_formatter/test.rs +++ b/helix-core/src/doc_formatter/test.rs @@ -6,7 +6,7 @@ impl TextFormat { TextFormat { soft_wrap: softwrap, tab_width: 2, - max_wrap: 3, + max_wrap: Some(3), max_indent_retain: 4, wrap_indicator: ".".into(), wrap_indicator_highlight: None, diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs index 3fcddfcd1..8ce154c89 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -32,7 +32,6 @@ pub mod text_annotations; pub mod textobject; mod transaction; pub mod uri; -pub mod wrap; pub mod unicode { pub use unicode_general_category as category; diff --git a/helix-core/src/wrap.rs b/helix-core/src/wrap.rs deleted file mode 100644 index 337b389ae..000000000 --- a/helix-core/src/wrap.rs +++ /dev/null @@ -1,11 +0,0 @@ -use smartstring::{LazyCompact, SmartString}; -use textwrap::{Options, WordSplitter::NoHyphenation}; - -/// Given a slice of text, return the text re-wrapped to fit it -/// within the given width. -pub fn reflow_hard_wrap(text: &str, text_width: usize) -> SmartString { - let options = Options::new(text_width) - .word_splitter(NoHyphenation) - .word_separator(textwrap::WordSeparator::AsciiSpace); - textwrap::refill(text, options).into() -} diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 4e912127c..a6563d5fe 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -7,6 +7,7 @@ use crate::job::Job; use super::*; use helix_core::command_line::{Args, Flag, Signature, Token, TokenKind}; +use helix_core::doc_formatter::DocumentFormatter; use helix_core::fuzzy::fuzzy_match; use helix_core::indent::MAX_INDENT; use helix_core::line_ending; @@ -2159,14 +2160,30 @@ fn reflow(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyho .unwrap_or_else(|| doc.text_width()); let rope = doc.text(); + let slice = rope.slice(..); + let format = TextFormat { + soft_wrap: true, + tab_width: 8, + max_wrap: None, + max_indent_retain: u16::try_from(text_width).unwrap_or(u16::MAX), + wrap_indicator: Box::from(""), + wrap_indicator_highlight: None, + viewport_width: u16::try_from(text_width).unwrap_or(u16::MAX), + soft_wrap_at_text_width: true, + }; + let annotations = TextAnnotations::default(); - let selection = doc.selection(view.id); - let transaction = Transaction::change_by_selection(rope, selection, |range| { - let fragment = range.fragment(rope.slice(..)); - let reflowed_text = helix_core::wrap::reflow_hard_wrap(&fragment, text_width); - - (range.from(), range.to(), Some(reflowed_text)) - }); + let mut changes = Vec::new(); + for selection in doc.selection(view.id) { + let mut formatter = DocumentFormatter::new_at_prev_checkpoint( + slice.slice(..selection.to()), + &format, + &annotations, + selection.from(), + ); + changes.append(&mut formatter.reflow(rope, doc.line_ending)); + } + let transaction = Transaction::change(rope, changes.into_iter()); doc.apply(&transaction, view.id); doc.append_changes_to_history(view); diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 2af1a054f..e09a3e74d 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -820,3 +820,39 @@ async fn macro_play_within_macro_record() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn test_reflow() -> anyhow::Result<()> { + test(( + "#[|This is a long line bla bla bla]#", + ":reflow 5", + "#[|This +is a +long +line +bla +bla +bla]#", + )) + .await?; + + test(( + "#[|Very_long_words_should_not_be_broken_by_hard_wrap]#", + ":reflow 2", + "#[|Very_long_words_should_not_be_broken_by_hard_wrap]#", + )) + .await?; + + test(( + "#[|Spaces are removed when wrapping]#", + ":reflow 2", + "#[|Spaces +are +removed +when +wrapping]#", + )) + .await?; + + Ok(()) +} diff --git a/helix-view/src/annotations/diagnostics.rs b/helix-view/src/annotations/diagnostics.rs index 7802ca637..cc38e9105 100644 --- a/helix-view/src/annotations/diagnostics.rs +++ b/helix-view/src/annotations/diagnostics.rs @@ -96,7 +96,7 @@ impl InlineDiagnosticsConfig { TextFormat { soft_wrap: true, tab_width: 4, - max_wrap: self.max_wrap.min(width / 4), + max_wrap: Some(self.max_wrap.min(width / 4)), max_indent_retain: 0, wrap_indicator: "".into(), wrap_indicator_highlight: None, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 41c9ee1ef..ed407918f 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -2211,7 +2211,7 @@ impl Document { TextFormat { soft_wrap: enable_soft_wrap && viewport_width > 10, tab_width, - max_wrap: max_wrap.min(viewport_width / 4), + max_wrap: Some(max_wrap.min(viewport_width / 4)), max_indent_retain: max_indent_retain.min(viewport_width * 2 / 5), // avoid spinning forever when the window manager // sets the size to something tiny From b6dc0573df78cf87751d5dd824823779b91d289d Mon Sep 17 00:00:00 2001 From: Rose Hogenson Date: Tue, 7 Jan 2025 20:36:45 -0800 Subject: [PATCH 2/6] Handle single-line comment prefixes in :reflow. --- helix-core/src/doc_formatter.rs | 14 +++++- helix-core/src/doc_formatter/test.rs | 1 + helix-term/src/commands/typed.rs | 5 +++ helix-term/tests/test/commands.rs | 55 +++++++++++++++++++++++ helix-view/src/annotations/diagnostics.rs | 1 + helix-view/src/document.rs | 1 + 6 files changed, 76 insertions(+), 1 deletion(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index 6c426354d..e841475b4 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -152,6 +152,7 @@ pub struct TextFormat { pub wrap_indicator_highlight: Option, pub viewport_width: u16, pub soft_wrap_at_text_width: bool, + pub continue_comments: Vec, } // test implementation is basically only used for testing or when softwrap is always disabled @@ -166,6 +167,7 @@ impl Default for TextFormat { viewport_width: 17, wrap_indicator_highlight: None, soft_wrap_at_text_width: false, + continue_comments: Vec::new(), } } } @@ -437,8 +439,18 @@ impl<'t> DocumentFormatter<'t> { fn find_indent<'a>(&self, line: usize, doc: RopeSlice<'a>) -> RopeSlice<'a> { let line_start = doc.line_to_char(line); - let indent_end = movement::skip_while(doc, line_start, |ch| matches!(ch, ' ' | '\t')) + let mut indent_end = movement::skip_while(doc, line_start, |ch| matches!(ch, ' ' | '\t')) .unwrap_or(line_start); + let slice = doc.slice(indent_end..); + if let Some(token) = self + .text_fmt + .continue_comments + .iter() + .filter(|token| slice.starts_with(token)) + .max_by_key(|x| x.len()) + { + indent_end += token.chars().count(); + } let indent_end = movement::skip_while(doc, indent_end, |ch| matches!(ch, ' ' | '\t')) .unwrap_or(indent_end); return doc.slice(line_start..indent_end); diff --git a/helix-core/src/doc_formatter/test.rs b/helix-core/src/doc_formatter/test.rs index 6cee5d3fb..e4b76bfb2 100644 --- a/helix-core/src/doc_formatter/test.rs +++ b/helix-core/src/doc_formatter/test.rs @@ -13,6 +13,7 @@ impl TextFormat { // use a prime number to allow lining up too often with repeat viewport_width: 17, soft_wrap_at_text_width: false, + continue_comments: Vec::new(), } } } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index a6563d5fe..48f595a92 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2170,6 +2170,11 @@ fn reflow(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyho wrap_indicator_highlight: None, viewport_width: u16::try_from(text_width).unwrap_or(u16::MAX), soft_wrap_at_text_width: true, + continue_comments: Vec::from( + doc.language_config() + .and_then(|config| config.comment_tokens.as_deref()) + .unwrap_or(&[]), + ), }; let annotations = TextAnnotations::default(); diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index e09a3e74d..b7eddaaf3 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -836,6 +836,61 @@ bla]#", )) .await?; + test(( + "// #[|This is a really long comment that we want to break onto multiple lines.]#", + ":lang rust:reflow 13", + "// #[|This is a +// really long +// comment that +// we want to +// break onto +// multiple +// lines.]#", + )) + .await?; + + test(( + "#[\t// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod +\t// tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim +\t// veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea +\t// commodo consequat. Duis aute irure dolor in reprehenderit in voluptate +\t// velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint +\t// occaecat cupidatat non proident, sunt in culpa qui officia deserunt +\t// mollit anim id est laborum. +|]#", + ":lang go:reflow 50", + "#[\t// Lorem ipsum dolor sit amet, +\t// consectetur adipiscing elit, sed do +\t// eiusmod +\t// tempor incididunt ut labore et dolore +\t// magna aliqua. Ut enim ad minim +\t// veniam, quis nostrud exercitation +\t// ullamco laboris nisi ut aliquip ex ea +\t// commodo consequat. Duis aute irure +\t// dolor in reprehenderit in voluptate +\t// velit esse cillum dolore eu fugiat +\t// nulla pariatur. Excepteur sint +\t// occaecat cupidatat non proident, sunt +\t// in culpa qui officia deserunt +\t// mollit anim id est laborum. +|]#", + )) + .await?; + + test(( + " // #[|This document has multiple lines that each need wrapping + + /// currently we wrap each line completely separately in order to preserve existing newlines.]#", + ":lang rust:reflow 40", + " // #[|This document has multiple lines + // that each need wrapping + + /// currently we wrap each line + /// completely separately in order to + /// preserve existing newlines.]#" + )) + .await?; + test(( "#[|Very_long_words_should_not_be_broken_by_hard_wrap]#", ":reflow 2", diff --git a/helix-view/src/annotations/diagnostics.rs b/helix-view/src/annotations/diagnostics.rs index cc38e9105..17e7ae85a 100644 --- a/helix-view/src/annotations/diagnostics.rs +++ b/helix-view/src/annotations/diagnostics.rs @@ -102,6 +102,7 @@ impl InlineDiagnosticsConfig { wrap_indicator_highlight: None, viewport_width: width, soft_wrap_at_text_width: true, + continue_comments: Vec::new(), } } } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index ed407918f..d90e0ff69 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -2221,6 +2221,7 @@ impl Document { .and_then(|theme| theme.find_scope_index("ui.virtual.wrap")) .map(Highlight), soft_wrap_at_text_width, + continue_comments: Vec::new(), } } From df264ffbb4328b8916034c317a1a3c35b5eadcb6 Mon Sep 17 00:00:00 2001 From: Rose Hogenson Date: Tue, 25 Feb 2025 16:15:41 -0800 Subject: [PATCH 3/6] Only insert hard-wraps at whitespace. This is a better behavior for :reflow since it prevents breaking URLs across lines. --- helix-core/src/doc_formatter.rs | 8 +++----- helix-core/src/doc_formatter/test.rs | 1 + helix-term/src/commands/typed.rs | 1 + helix-term/tests/test/commands.rs | 4 ++-- helix-view/src/annotations/diagnostics.rs | 1 + helix-view/src/document.rs | 1 + 6 files changed, 9 insertions(+), 7 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index e841475b4..8ab4cdfda 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -136,10 +136,6 @@ impl<'a> GraphemeWithSource<'a> { fn width(&self) -> usize { self.grapheme.width() } - - fn is_word_boundary(&self) -> bool { - self.grapheme.is_word_boundary() - } } #[derive(Debug, Clone)] @@ -153,6 +149,7 @@ pub struct TextFormat { pub viewport_width: u16, pub soft_wrap_at_text_width: bool, pub continue_comments: Vec, + pub is_word_boundary: fn(&Grapheme) -> bool, } // test implementation is basically only used for testing or when softwrap is always disabled @@ -168,6 +165,7 @@ impl Default for TextFormat { wrap_indicator_highlight: None, soft_wrap_at_text_width: false, continue_comments: Vec::new(), + is_word_boundary: |g| g.is_word_boundary(), } } } @@ -418,7 +416,7 @@ impl<'t> DocumentFormatter<'t> { self.indent_level = None; } - let is_word_boundary = grapheme.is_word_boundary(); + let is_word_boundary = (self.text_fmt.is_word_boundary)(&grapheme.grapheme); word_width += grapheme.width(); self.word_buf.push(grapheme); diff --git a/helix-core/src/doc_formatter/test.rs b/helix-core/src/doc_formatter/test.rs index e4b76bfb2..4e7351768 100644 --- a/helix-core/src/doc_formatter/test.rs +++ b/helix-core/src/doc_formatter/test.rs @@ -14,6 +14,7 @@ impl TextFormat { viewport_width: 17, soft_wrap_at_text_width: false, continue_comments: Vec::new(), + is_word_boundary: |g| g.is_word_boundary(), } } } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 48f595a92..1c8ee546e 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2175,6 +2175,7 @@ fn reflow(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyho .and_then(|config| config.comment_tokens.as_deref()) .unwrap_or(&[]), ), + is_word_boundary: |g| g.is_whitespace(), }; let annotations = TextAnnotations::default(); diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index b7eddaaf3..c59873ef5 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -892,9 +892,9 @@ bla]#", .await?; test(( - "#[|Very_long_words_should_not_be_broken_by_hard_wrap]#", + "#[|Very-long-words-should-not-be-broken-by-hard-wrap]#", ":reflow 2", - "#[|Very_long_words_should_not_be_broken_by_hard_wrap]#", + "#[|Very-long-words-should-not-be-broken-by-hard-wrap]#", )) .await?; diff --git a/helix-view/src/annotations/diagnostics.rs b/helix-view/src/annotations/diagnostics.rs index 17e7ae85a..e02ce34a3 100644 --- a/helix-view/src/annotations/diagnostics.rs +++ b/helix-view/src/annotations/diagnostics.rs @@ -103,6 +103,7 @@ impl InlineDiagnosticsConfig { viewport_width: width, soft_wrap_at_text_width: true, continue_comments: Vec::new(), + is_word_boundary: |g| g.is_word_boundary(), } } } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index d90e0ff69..5a1d51ddb 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -2222,6 +2222,7 @@ impl Document { .map(Highlight), soft_wrap_at_text_width, continue_comments: Vec::new(), + is_word_boundary: |g| g.is_word_boundary(), } } From 8566d5ca1a809bb009c0db16f04a33539d5518c1 Mon Sep 17 00:00:00 2001 From: Rose Hogenson Date: Sun, 30 Mar 2025 11:31:20 -0700 Subject: [PATCH 4/6] Separate reflow from DocumentFormatter It was making things much more complicated by needing to handle two different cases in the same code path. By separating reflow out, it can be more efficient and easier to debug. --- helix-core/src/doc_formatter.rs | 141 +++++++++++++++++++----------- helix-term/src/commands/typed.rs | 33 +++---- helix-term/tests/test/commands.rs | 17 ++-- 3 files changed, 109 insertions(+), 82 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index 8ab4cdfda..4145ce7a9 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -24,7 +24,7 @@ use helix_stdx::rope::{RopeGraphemes, RopeSliceExt}; use crate::graphemes::{Grapheme, GraphemeStr}; use crate::syntax::Highlight; use crate::text_annotations::TextAnnotations; -use crate::{movement, Change, LineEnding, Position, Rope, RopeSlice, Tendril}; +use crate::{movement, Change, LineEnding, Position, RopeSlice, Tendril}; /// TODO make Highlight a u32 to reduce the size of this enum to a single word. #[derive(Debug, Clone, Copy)] @@ -434,57 +434,6 @@ impl<'t> DocumentFormatter<'t> { pub fn next_visual_pos(&self) -> Position { self.visual_pos } - - fn find_indent<'a>(&self, line: usize, doc: RopeSlice<'a>) -> RopeSlice<'a> { - let line_start = doc.line_to_char(line); - let mut indent_end = movement::skip_while(doc, line_start, |ch| matches!(ch, ' ' | '\t')) - .unwrap_or(line_start); - let slice = doc.slice(indent_end..); - if let Some(token) = self - .text_fmt - .continue_comments - .iter() - .filter(|token| slice.starts_with(token)) - .max_by_key(|x| x.len()) - { - indent_end += token.chars().count(); - } - let indent_end = movement::skip_while(doc, indent_end, |ch| matches!(ch, ' ' | '\t')) - .unwrap_or(indent_end); - return doc.slice(line_start..indent_end); - } - - /// consumes the iterator and hard-wraps the input where soft wraps would - /// have been applied. It probably only makes sense to call this method if - /// soft_wrap is true. - pub fn reflow(&mut self, doc: &Rope, line_ending: LineEnding) -> Vec { - let slice = doc.slice(..); - let mut current_line = self.visual_pos.row; - let mut changes = Vec::new(); - while let Some(grapheme) = self.next() { - if !grapheme.is_whitespace() && grapheme.visual_pos.row != current_line { - let indent = Tendril::from(format!( - "{}{}", - line_ending.as_str(), - self.find_indent(doc.char_to_line(grapheme.char_idx - 1), slice) - )); - let mut whitespace_start = grapheme.char_idx; - let mut whitespace_end = grapheme.char_idx; - while whitespace_start > 0 && slice.char(whitespace_start - 1) == ' ' { - whitespace_start -= 1; - } - while whitespace_end < slice.chars().len() && slice.char(whitespace_end) == ' ' { - whitespace_end += 1; - } - changes.push((whitespace_start, whitespace_end, Some(indent))); - current_line = grapheme.visual_pos.row; - } - if grapheme.raw == Grapheme::Newline { - current_line += 1; - } - } - changes - } } impl<'t> Iterator for DocumentFormatter<'t> { @@ -535,3 +484,91 @@ impl<'t> Iterator for DocumentFormatter<'t> { Some(grapheme) } } + +pub struct ReflowOpts<'a> { + pub width: usize, + pub line_ending: LineEnding, + pub comment_tokens: &'a [String], +} + +impl ReflowOpts<'_> { + fn find_indent<'a>(&self, line: usize, doc: RopeSlice<'a>) -> RopeSlice<'a> { + let line_start = doc.line_to_char(line); + let mut indent_end = movement::skip_while(doc, line_start, |ch| matches!(ch, ' ' | '\t')) + .unwrap_or(line_start); + let slice = doc.slice(indent_end..); + if let Some(token) = self + .comment_tokens + .iter() + .filter(|token| slice.starts_with(token)) + .max_by_key(|x| x.len()) + { + indent_end += token.chars().count(); + } + let indent_end = movement::skip_while(doc, indent_end, |ch| matches!(ch, ' ' | '\t')) + .unwrap_or(indent_end); + return doc.slice(line_start..indent_end); + } +} + +/// reflow wraps long lines in text to be less than opts.width. +pub fn reflow(text: RopeSlice, char_pos: usize, opts: &ReflowOpts) -> Vec { + // A constant so that reflow behaves consistently across + // different configurations. + const TAB_WIDTH: u16 = 8; + + let line_idx = text.char_to_line(char_pos.min(text.len_chars())); + let mut char_pos = text.line_to_char(line_idx); + + let mut col = 0; + let mut word_width = 0; + let mut last_word_boundary = None; + let mut changes = Vec::new(); + for grapheme in text.graphemes() { + let grapheme_chars = grapheme.len_chars(); + let mut grapheme = Grapheme::new(GraphemeStr::from(Cow::from(grapheme)), col, TAB_WIDTH); + if col + grapheme.width() > opts.width && !grapheme.is_whitespace() { + if let Some(n) = last_word_boundary { + let indent = opts.find_indent(text.char_to_line(n - 1), text); + let mut whitespace_start = n; + let mut whitespace_end = n; + while whitespace_start > 0 && text.char(whitespace_start - 1) == ' ' { + whitespace_start -= 1; + } + while whitespace_end < text.chars().len() && text.char(whitespace_end) == ' ' { + whitespace_end += 1; + } + changes.push(( + whitespace_start, + whitespace_end, + Some(Tendril::from(format!( + "{}{}", + opts.line_ending.as_str(), + &indent + ))), + )); + + col = 0; + for g in indent.graphemes() { + let g = Grapheme::new(GraphemeStr::from(Cow::from(g)), col, TAB_WIDTH); + col += g.width(); + } + col += word_width; + last_word_boundary = None; + grapheme.change_position(col, TAB_WIDTH); + } + } + col += grapheme.width(); + word_width += grapheme.width(); + if grapheme == Grapheme::Newline { + col = 0; + word_width = 0; + last_word_boundary = None; + } else if grapheme.is_whitespace() { + last_word_boundary = Some(char_pos); + word_width = 0; + } + char_pos += grapheme_chars; + } + changes +} diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 1c8ee546e..f5c8e0798 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -7,7 +7,7 @@ use crate::job::Job; use super::*; use helix_core::command_line::{Args, Flag, Signature, Token, TokenKind}; -use helix_core::doc_formatter::DocumentFormatter; +use helix_core::doc_formatter::ReflowOpts; use helix_core::fuzzy::fuzzy_match; use helix_core::indent::MAX_INDENT; use helix_core::line_ending; @@ -2161,33 +2161,22 @@ fn reflow(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyho let rope = doc.text(); let slice = rope.slice(..); - let format = TextFormat { - soft_wrap: true, - tab_width: 8, - max_wrap: None, - max_indent_retain: u16::try_from(text_width).unwrap_or(u16::MAX), - wrap_indicator: Box::from(""), - wrap_indicator_highlight: None, - viewport_width: u16::try_from(text_width).unwrap_or(u16::MAX), - soft_wrap_at_text_width: true, - continue_comments: Vec::from( - doc.language_config() - .and_then(|config| config.comment_tokens.as_deref()) - .unwrap_or(&[]), - ), - is_word_boundary: |g| g.is_whitespace(), + let opts = ReflowOpts { + width: text_width, + line_ending: doc.line_ending, + comment_tokens: doc + .language_config() + .and_then(|config| config.comment_tokens.as_deref()) + .unwrap_or(&[]), }; - let annotations = TextAnnotations::default(); let mut changes = Vec::new(); for selection in doc.selection(view.id) { - let mut formatter = DocumentFormatter::new_at_prev_checkpoint( + changes.append(&mut helix_core::doc_formatter::reflow( slice.slice(..selection.to()), - &format, - &annotations, selection.from(), - ); - changes.append(&mut formatter.reflow(rope, doc.line_ending)); + &opts, + )); } let transaction = Transaction::change(rope, changes.into_iter()); diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index c59873ef5..f1b4d23c0 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -840,9 +840,11 @@ bla]#", "// #[|This is a really long comment that we want to break onto multiple lines.]#", ":lang rust:reflow 13", "// #[|This is a -// really long -// comment that -// we want to +// really +// long +// comment +// that we +// want to // break onto // multiple // lines.]#", @@ -859,9 +861,8 @@ bla]#", \t// mollit anim id est laborum. |]#", ":lang go:reflow 50", - "#[\t// Lorem ipsum dolor sit amet, -\t// consectetur adipiscing elit, sed do -\t// eiusmod + "#[\t// Lorem ipsum dolor sit amet, consectetur +\t// adipiscing elit, sed do eiusmod \t// tempor incididunt ut labore et dolore \t// magna aliqua. Ut enim ad minim \t// veniam, quis nostrud exercitation @@ -886,8 +887,8 @@ bla]#", // that each need wrapping /// currently we wrap each line - /// completely separately in order to - /// preserve existing newlines.]#" + /// completely separately in order + /// to preserve existing newlines.]#" )) .await?; From aed244f1275cd6d262a3e672c1451f36e09ca11d Mon Sep 17 00:00:00 2001 From: Rose Hogenson Date: Sun, 30 Mar 2025 15:40:24 -0700 Subject: [PATCH 5/6] Remove unnecessary fields --- helix-core/src/doc_formatter.rs | 28 +++++++++-------------- helix-core/src/doc_formatter/test.rs | 4 +--- helix-term/src/commands/typed.rs | 3 +-- helix-view/src/annotations/diagnostics.rs | 4 +--- helix-view/src/document.rs | 4 +--- 5 files changed, 15 insertions(+), 28 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index 4145ce7a9..70fe2f15e 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -136,20 +136,22 @@ impl<'a> GraphemeWithSource<'a> { fn width(&self) -> usize { self.grapheme.width() } + + fn is_word_boundary(&self) -> bool { + self.grapheme.is_word_boundary() + } } #[derive(Debug, Clone)] pub struct TextFormat { pub soft_wrap: bool, pub tab_width: u16, - pub max_wrap: Option, + pub max_wrap: u16, pub max_indent_retain: u16, pub wrap_indicator: Box, pub wrap_indicator_highlight: Option, pub viewport_width: u16, pub soft_wrap_at_text_width: bool, - pub continue_comments: Vec, - pub is_word_boundary: fn(&Grapheme) -> bool, } // test implementation is basically only used for testing or when softwrap is always disabled @@ -158,14 +160,12 @@ impl Default for TextFormat { TextFormat { soft_wrap: false, tab_width: 4, - max_wrap: Some(3), + max_wrap: 3, max_indent_retain: 4, wrap_indicator: Box::from(" "), viewport_width: 17, wrap_indicator_highlight: None, soft_wrap_at_text_width: false, - continue_comments: Vec::new(), - is_word_boundary: |g| g.is_word_boundary(), } } } @@ -386,22 +386,16 @@ impl<'t> DocumentFormatter<'t> { .peek_grapheme(col, char_pos) .is_some_and(|grapheme| grapheme.is_newline() || grapheme.is_eof()) => { } - Ordering::Equal - if word_width > self.text_fmt.max_wrap.map_or(usize::MAX, usize::from) => - { - return; - } - Ordering::Greater - if word_width > self.text_fmt.max_wrap.map_or(usize::MAX, usize::from) => - { + Ordering::Equal if word_width > self.text_fmt.max_wrap as usize => return, + Ordering::Greater if word_width > self.text_fmt.max_wrap as usize => { self.peeked_grapheme = self.word_buf.pop(); return; } - Ordering::Equal | Ordering::Greater if self.visual_pos.col > 0 => { + Ordering::Equal | Ordering::Greater => { word_width = self.wrap_word(); col = self.visual_pos.col + word_width; } - _ => (), + Ordering::Less => (), } let Some(grapheme) = self.next_grapheme(col, char_pos) else { @@ -416,7 +410,7 @@ impl<'t> DocumentFormatter<'t> { self.indent_level = None; } - let is_word_boundary = (self.text_fmt.is_word_boundary)(&grapheme.grapheme); + let is_word_boundary = grapheme.is_word_boundary(); word_width += grapheme.width(); self.word_buf.push(grapheme); diff --git a/helix-core/src/doc_formatter/test.rs b/helix-core/src/doc_formatter/test.rs index 4e7351768..21be2e535 100644 --- a/helix-core/src/doc_formatter/test.rs +++ b/helix-core/src/doc_formatter/test.rs @@ -6,15 +6,13 @@ impl TextFormat { TextFormat { soft_wrap: softwrap, tab_width: 2, - max_wrap: Some(3), + max_wrap: 3, max_indent_retain: 4, wrap_indicator: ".".into(), wrap_indicator_highlight: None, // use a prime number to allow lining up too often with repeat viewport_width: 17, soft_wrap_at_text_width: false, - continue_comments: Vec::new(), - is_word_boundary: |g| g.is_word_boundary(), } } } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index f5c8e0798..6d6a2ce00 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2160,7 +2160,6 @@ fn reflow(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyho .unwrap_or_else(|| doc.text_width()); let rope = doc.text(); - let slice = rope.slice(..); let opts = ReflowOpts { width: text_width, line_ending: doc.line_ending, @@ -2173,7 +2172,7 @@ fn reflow(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyho let mut changes = Vec::new(); for selection in doc.selection(view.id) { changes.append(&mut helix_core::doc_formatter::reflow( - slice.slice(..selection.to()), + rope.slice(..selection.to()), selection.from(), &opts, )); diff --git a/helix-view/src/annotations/diagnostics.rs b/helix-view/src/annotations/diagnostics.rs index e02ce34a3..7802ca637 100644 --- a/helix-view/src/annotations/diagnostics.rs +++ b/helix-view/src/annotations/diagnostics.rs @@ -96,14 +96,12 @@ impl InlineDiagnosticsConfig { TextFormat { soft_wrap: true, tab_width: 4, - max_wrap: Some(self.max_wrap.min(width / 4)), + max_wrap: self.max_wrap.min(width / 4), max_indent_retain: 0, wrap_indicator: "".into(), wrap_indicator_highlight: None, viewport_width: width, soft_wrap_at_text_width: true, - continue_comments: Vec::new(), - is_word_boundary: |g| g.is_word_boundary(), } } } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 5a1d51ddb..41c9ee1ef 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -2211,7 +2211,7 @@ impl Document { TextFormat { soft_wrap: enable_soft_wrap && viewport_width > 10, tab_width, - max_wrap: Some(max_wrap.min(viewport_width / 4)), + max_wrap: max_wrap.min(viewport_width / 4), max_indent_retain: max_indent_retain.min(viewport_width * 2 / 5), // avoid spinning forever when the window manager // sets the size to something tiny @@ -2221,8 +2221,6 @@ impl Document { .and_then(|theme| theme.find_scope_index("ui.virtual.wrap")) .map(Highlight), soft_wrap_at_text_width, - continue_comments: Vec::new(), - is_word_boundary: |g| g.is_word_boundary(), } } From 4d64d670dd6a6df693df2fc4563992d9c5f7756a Mon Sep 17 00:00:00 2001 From: Rose Hogenson Date: Mon, 31 Mar 2025 12:16:36 -0700 Subject: [PATCH 6/6] Fix grapheme indexing --- helix-core/src/doc_formatter.rs | 2 +- helix-term/tests/test/commands.rs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index 70fe2f15e..bebda0535 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -518,7 +518,7 @@ pub fn reflow(text: RopeSlice, char_pos: usize, opts: &ReflowOpts) -> Vec opts.width && !grapheme.is_whitespace() { diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index f1b4d23c0..4dc94d38a 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -910,5 +910,22 @@ wrapping]#", )) .await?; + test(( + "Test wrapping only part of the text +#[|wrapping should only modify the lines that are currently selected +]#", + ":reflow 11", + "Test wrapping only part of the text +#[|wrapping +should only +modify the +lines that +are +currently +selected +]#", + )) + .await?; + Ok(()) }