Skip to content

Correctly justify with consecutive trailing spaces - #760

Merged
tomcur merged 2 commits into
linebender:mainfrom
tomcur:justify-trailing-whitespace
Sep 4, 2026
Merged

Correctly justify with consecutive trailing spaces#760
tomcur merged 2 commits into
linebender:mainfrom
tomcur:justify-trailing-whitespace

Conversation

@tomcur

@tomcur tomcur commented Aug 31, 2026

Copy link
Copy Markdown
Member

LLM Contributions: Investigation, test.

When justifying a line, this now correctly handles runs of consecutive trailing spaces. This is follow-up to #738 (comment).

There's some more that we could/should do here (separately!). E.g., I think we simply do not need to check the break reason, and that would also get us to handle BreakReason::Emergency correctly for free. We also don't currently handle consecutive trailing spaces correctly for either justification or hanging when there's a formatting change inside that run; something like the following.

<div style="background: #efefef; width: 95px; white-space-collapse: preserve; text-align: justify;">AA BB CC  <span style="font-size: 2em;"> </span>DD</div>

Changelog

Fixed

  • Lines aligned with justification now correctly handle runs of consecutive trailing spaces.

@tomcur tomcur left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the fix, the snapshot looks as follows.

Image

@tomcur
tomcur force-pushed the justify-trailing-whitespace branch 2 times, most recently from 989f22c to 8447b49 Compare September 3, 2026 12:26
@tomcur
tomcur force-pushed the justify-trailing-whitespace branch from 8447b49 to 2072777 Compare September 4, 2026 14:13
@tomcur
tomcur added this pull request to the merge queue Sep 4, 2026
Merged via the queue into linebender:main with commit 4ce3f64 Sep 4, 2026
24 checks passed
@tomcur
tomcur deleted the justify-trailing-whitespace branch September 4, 2026 14:36

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see this fixed. We've been carrying a patch for this for quite a while 😅

let mut layout = builder.build(text);
layout.break_all_lines(Some(max_advance));
layout.align(Alignment::Justify, AlignmentOptions::default());
env.check_cluster_snapshot(&layout, text, 12.0);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know this existed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants