[DO NOT MERGE] Hang trailing whitespace (conditionally for last line in paragraph)#3
[DO NOT MERGE] Hang trailing whitespace (conditionally for last line in paragraph)#3conor-93 wants to merge 1 commit into
Conversation
…t of overflow). Add visreg tests for this
conor-93
changed the title
djmcnab-canva
left a comment
djmcnab-canva
left a comment
There was a problem hiding this comment.
This looks pretty good. I definitely understand how this resolves.
One question: Did you evaluate using linebender#485 instead? In my reading, that looks like it solves the same issues with very similar approaches, but also handles the hanging clusters issue.
| // Hanging whitespace is an alignment/positioning concern, orthogonal to text | ||
| // directionality. The RTL handling here doesn't interact with bidi reordering: | ||
| // it simply shifts the line's origin so that hung trailing whitespace (which | ||
| // sits at the *start* edge in visual order for RTL) overflows into the start | ||
| // margin rather than displacing visible content. |
There was a problem hiding this comment.
This smells like a Claude artifact to me. I'd lean towards reverting this section.
| // TODO: can this be hoisted out of the conditional? | ||
| self.state.cluster_idx += 1; |
There was a problem hiding this comment.
I'm slightly surprised that this actually gets all the remaining space clusters in the line; my impression was previously that the reason we were seeing the wrapping behaviour was because this is +1. That is, do we not need to consume clusters until we reach the last cluster on the line? We might need to do more digging
There was a problem hiding this comment.
It isn't clear to me why this snapshot has been regenerated. Do you have that clear?
I can't see any actual changes, so I'm not overly worried.
2 participants
No description provided.