Skip to content

Reset contributed in BreakerState::reset_line - #854

Merged
nicoburns merged 2 commits into
linebender:mainfrom
DioxusLabs:devin/1790619679-reset-line-buffers
Sep 29, 2026
Merged

nicoburns merged 2 commits into
linebender:mainfrom
DioxusLabs:devin/1790619679-reset-line-buffers

Conversation

@nicoburns

Copy link
Copy Markdown
Collaborator

LLM Contributions: Generated with Opus 5.5 High. Reviewed by me.

This is a small refactor motivated by code quality. Previously an &mut reference to contributed was being passed into LineBoxMetrics's reset method. This is a violation of separation of concerns. Resetting contributed now happens in the reset_line method of BreakerState which actually owns contributed.

Changelog: None (pure refactor)

@nicoburns
nicoburns requested review from DJMcNab and tomcur and a lite review from Copilot and removed request for Copilot September 28, 2026 20:35

@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.

Reset applying the strut also doesn't read great here.

That is, I think reset_line should be the caller of add_strut (although I'm also not entirely convinced by add_strut; it seems like the strut should arise naturally from adding a box from a style - especially if lines with only inline boxes don't want the struts anyway...)

@staging-devin-ai-integration
staging-devin-ai-integration Bot force-pushed the devin/1790619679-reset-line-buffers branch from e614d8f to 1c869cb Compare September 29, 2026 12:58
@nicoburns

Copy link
Copy Markdown
Collaborator Author

especially if lines with only inline boxes don't want the struts anyway...

FWIW, I believe that lines with only inline boxes do want the strut

@nicoburns
nicoburns enabled auto-merge September 29, 2026 13:02
@nicoburns
nicoburns added this pull request to the merge queue Sep 29, 2026
Merged via the queue into linebender:main with commit 30b3055 Sep 29, 2026
24 checks passed
@DJMcNab

DJMcNab commented Sep 29, 2026

Copy link
Copy Markdown
Member

FWIW, I believe that lines with only inline boxes do want the strut

Hmm, I thought this was the logic about lines with absolutely no glyphs. I guess I'm still confused about that change!
But sure, that isn't too important for us.

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.

2 participants