Skip to content

Commit

Permalink
Account for inline boxes when collapsing after newlines (#280)
Browse files Browse the repository at this point in the history
Followup (small bug fix) to
#254 which didn't take inline
boxes into account. That PR was checking whether "the last character is
whitespace" without checking whether the last thing pushed into the
layout was text.

So if the sequence of items pushed to a layout was something like:

- inline box
- whitespace
- inline box
- whitespace
- inline box
- whitespace
- inline box

(an example which would be "status badges" in Github readme's which are
typically images separated by whitespace characters)

Then all but the first "whitespace" would get collapsed as it would be
incorrectly detected as directly adjacent to the preceding whitespace
(even though it shouldn't because there is an inline box in between).

This PR fixes this issue by tracking the kind of the last item pushed.

---------

Signed-off-by: Nico Burns <[email protected]>
  • Loading branch information
nicoburns authored Feb 20, 2025
1 parent 5f3988b commit 7c8983c
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 5 deletions.
4 changes: 4 additions & 0 deletions parley/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use alloc::string::String;
use core::ops::RangeBounds;

use crate::inline_box::InlineBox;
use crate::resolve::tree::ItemKind;

/// Builder for constructing a text layout with ranged attributes.
pub struct RangedBuilder<'a, B: Brush> {
Expand Down Expand Up @@ -102,6 +103,9 @@ impl<B: Brush> TreeBuilder<'_, B> {
pub fn push_inline_box(&mut self, mut inline_box: InlineBox) {
self.lcx.tree_style_builder.push_uncommitted_text(false);
self.lcx.tree_style_builder.set_is_span_first(false);
self.lcx
.tree_style_builder
.set_last_item_kind(ItemKind::InlineBox);
// TODO: arrange type better here to factor out the index
inline_box.index = self.lcx.tree_style_builder.current_text_len();
self.lcx.inline_boxes.push(inline_box);
Expand Down
25 changes: 20 additions & 5 deletions parley/src/resolve/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ struct StyleTreeNode<B: Brush> {
style: ResolvedStyle<B>,
}

#[derive(Clone, Copy, PartialEq)]
pub(crate) enum ItemKind {
None,
InlineBox,
TextRun,
}

/// Builder for constructing a tree of styles
#[derive(Clone)]
pub(crate) struct TreeStyleBuilder<B: Brush> {
Expand All @@ -25,6 +32,7 @@ pub(crate) struct TreeStyleBuilder<B: Brush> {
uncommitted_text: String,
current_span: usize,
is_span_first: bool,
last_item_kind: ItemKind,
}

impl<B: Brush> TreeStyleBuilder<B> {
Expand All @@ -43,6 +51,7 @@ impl<B: Brush> Default for TreeStyleBuilder<B> {
uncommitted_text: String::new(),
current_span: usize::MAX,
is_span_first: false,
last_item_kind: ItemKind::None,
}
}
}
Expand Down Expand Up @@ -72,18 +81,23 @@ impl<B: Brush> TreeStyleBuilder<B> {
self.is_span_first = is_span_first;
}

pub(crate) fn set_last_item_kind(&mut self, item_kind: ItemKind) {
self.last_item_kind = item_kind;
}

pub(crate) fn push_uncommitted_text(&mut self, is_span_last: bool) {
let span_text: Cow<'_, str> = match self.white_space_collapse {
WhiteSpaceCollapse::Preserve => Cow::from(&self.uncommitted_text),
WhiteSpaceCollapse::Collapse => {
let mut span_text = self.uncommitted_text.as_str();

if self.is_span_first
|| self
.text
.chars()
.last()
.is_some_and(|c| c.is_ascii_whitespace())
|| (self.last_item_kind == ItemKind::TextRun
&& self
.text
.chars()
.last()
.is_some_and(|c| c.is_ascii_whitespace()))
{
span_text = span_text.trim_start();
}
Expand Down Expand Up @@ -129,6 +143,7 @@ impl<B: Brush> TreeStyleBuilder<B> {
self.text.push_str(span_text);
self.uncommitted_text.clear();
self.is_span_first = false;
self.last_item_kind = ItemKind::TextRun;
}

pub(crate) fn current_text_len(&self) -> usize {
Expand Down
38 changes: 38 additions & 0 deletions parley/src/tests/test_basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,44 @@ fn full_width_inbox() {
}
}

#[test]
fn inbox_separated_by_whitespace() {
let mut env = testenv!();

let mut builder = env.tree_builder();
builder.push_inline_box(InlineBox {
id: 0,
index: 0,
width: 10.,
height: 10.0,
});
builder.push_text(" ");
builder.push_inline_box(InlineBox {
id: 1,
index: 1,
width: 10.0,
height: 10.0,
});
builder.push_text(" ");
builder.push_inline_box(InlineBox {
id: 2,
index: 2,
width: 10.0,
height: 10.0,
});
builder.push_text(" ");
builder.push_inline_box(InlineBox {
id: 3,
index: 3,
width: 10.0,
height: 10.0,
});
let (mut layout, _text) = builder.build();
layout.break_all_lines(Some(100.));
layout.align(None, Alignment::Start, AlignmentOptions::default());
env.check_layout_snapshot(&layout);
}

#[test]
fn trailing_whitespace() {
let mut env = testenv!();
Expand Down
3 changes: 3 additions & 0 deletions parley/tests/snapshots/inbox_separated_by_whitespace-0.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 7c8983c

Please sign in to comment.