Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Calculate minimum and maximum content width #259
Calculate minimum and maximum content width #259
Changes from all commits
14a5168
1fc33c3
ef77c4d
3109048
5c43008
9306242
972f041
3650ee6
acf421f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if
greedy.rs
can be rewritten to useBoundary::Mandatory
rather than checking for newline white space.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe @dfrg can shine some light on this, but from my quick look at Swash, I assume that
Boundary::Mandatory
corresponds the mandatory break in the Unicode standard here (see also Table 1), which does seem to be the correct choice for line breaking as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It used to! This was changed in the recent refactor of that code to fix selection/cursors. I can't remember why it was changed, but I believe it was a matter of convenience as part of a wider refactor and that it ought to be possible to change it back to using
Boundary::Mandatory
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swash follows the Unicode LBA and attaches
Boundary::Mandatory
to the cluster after the newline sequence which means we never encounter that state when the source text ends with a newline. This is arguably a swash bug but I “fixed” it in parley by just checking for the newline white space flag and that actually simplified a lot of code.