Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent soft wrap inside a run of punctuation #12959

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 23 additions & 17 deletions helix-core/src/doc_formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -170,6 +166,10 @@ impl Default for TextFormat {
}
}

fn wrap_allowed(x: Option<&Grapheme>, y: &Grapheme) -> bool {
x.is_some_and(|x| x.is_word_boundary() && (x.is_whitespace() || !y.is_word_boundary()))
}

#[derive(Debug)]
pub struct DocumentFormatter<'t> {
text_fmt: &'t TextFormat,
Expand Down Expand Up @@ -352,17 +352,12 @@ impl<'t> DocumentFormatter<'t> {
self.peeked_grapheme.as_ref()
}

fn next_grapheme(&mut self, col: usize, char_pos: usize) -> Option<GraphemeWithSource<'t>> {
self.peek_grapheme(col, char_pos);
self.peeked_grapheme.take()
}

fn advance_to_next_word(&mut self) {
self.word_buf.clear();
let mut word_width = 0;
let mut word_chars = 0;

if self.exhausted {
if self.exhausted && self.peeked_grapheme.is_none() {
return;
}

Expand All @@ -384,7 +379,10 @@ impl<'t> DocumentFormatter<'t> {
if self.text_fmt.soft_wrap_at_text_width
&& self
.peek_grapheme(col, char_pos)
.is_some_and(|grapheme| grapheme.is_newline() || grapheme.is_eof()) => {
.is_some_and(|grapheme| grapheme.is_newline() || grapheme.is_eof()) =>
{
self.word_buf.push(self.peeked_grapheme.take().unwrap());
return;
}
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 => {
Expand All @@ -398,9 +396,22 @@ impl<'t> DocumentFormatter<'t> {
Ordering::Less => (),
}

let Some(grapheme) = self.next_grapheme(col, char_pos) else {
// It would be nice if this could be written as
// let Some(grapheme) = self.peek_grapheme(col, char_pos)
// but the borrow checker can't see inside of peek_grapheme and
// assumes that the return value borrows mutably from self.
let _ = self.peek_grapheme(col, char_pos);
let Some(grapheme) = &self.peeked_grapheme else {
return;
};
if wrap_allowed(
self.word_buf.last().map(|g| &g.grapheme),
&grapheme.grapheme,
) {
return;
}

let grapheme = self.peeked_grapheme.take().unwrap();
word_chars += grapheme.doc_chars();

// Track indentation
Expand All @@ -410,13 +421,8 @@ impl<'t> DocumentFormatter<'t> {
self.indent_level = None;
}

let is_word_boundary = grapheme.is_word_boundary();
word_width += grapheme.width();
self.word_buf.push(grapheme);

if is_word_boundary {
return;
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions helix-core/src/doc_formatter/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,18 @@ fn softwrap_multichar_grapheme() {
)
}

#[test]
fn softwrap_punctuation() {
assert_eq!(
softwrap_text("asdfasdfasdfasd ...\n"),
"asdfasdfasdfasd \n.... \n "
);
assert_eq!(
softwrap_text("asdfasdfasdf a(bc)\n"),
"asdfasdfasdf a(\n.bc) \n "
);
}

fn softwrap_text_at_text_width(text: &str) -> String {
let mut text_fmt = TextFormat::new_test(true);
text_fmt.soft_wrap_at_text_width = true;
Expand Down