Skip to content
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

Make alignment when free space is negative configurable #241

Merged
merged 4 commits into from
Jan 22, 2025

Conversation

nicoburns
Copy link
Contributor

Untested, but probably fixes #240

@nicoburns nicoburns added the bug Something isn't working label Jan 17, 2025
@nicoburns nicoburns changed the title Fix alignment when free_space is negative Fix alignment when free space is negative Jan 17, 2025
@nicoburns nicoburns force-pushed the negative-space-alignment branch from fb70aeb to 5c855df Compare January 17, 2025 11:34
@nicoburns nicoburns requested a review from tomcur January 21, 2025 09:36
@nicoburns nicoburns changed the title Fix alignment when free space is negative Make alignment when free space is negative configurable Jan 21, 2025
@nicoburns nicoburns force-pushed the negative-space-alignment branch from c762f49 to 4ec40e5 Compare January 21, 2025 09:47
Copy link
Member

@tomcur tomcur left a comment

Choose a reason for hiding this comment

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

This looks good to me, but see my note about the boolean option. Perhaps we want one more person to agree this is sensible to have.

A quick test of single words that overflow the width in vello_editor works as expected.

layout.align(
max_advance,
Alignment::Start,
false, /* align_when_overflowing */
Copy link
Member

Choose a reason for hiding this comment

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

If align gains more options in the future, a boolean may be a bit confusing, especially if people don't document as you've done here. Maybe an AlignmentOptions struct could be introduced?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, but don't have time to work on this now. In the interest of getting this landed for Parley 0.3 I'm going to merge this as-is. And have created an issue for the options struct (#247)

Signed-off-by: Nico Burns <[email protected]>
@nicoburns nicoburns added this pull request to the merge queue Jan 22, 2025
Merged via the queue into linebender:main with commit 0f90765 Jan 22, 2025
20 checks passed
@nicoburns nicoburns deleted the negative-space-alignment branch January 22, 2025 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alignment of long text without word wrap
2 participants