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

Fix bug with text wrapping past max width #434

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

msvbg
Copy link
Contributor

@msvbg msvbg commented Apr 5, 2019

TextWrapping.wrapText can throw an exception when trying to wrap text that's too wide for the container. To be fair, I don't totally understand the code here so I may have messed something up. But the supplied test case throws an exception without this change to TextWrapping.re, so that's something at least.

On another note, it may be worthwhile to look at Jane Street's Base instead of the Ocaml standard library, which throws exceptions like String.sub does here.

@msvbg msvbg force-pushed the text-wrapping-bug branch 2 times, most recently from 95fcb64 to 72c20c0 Compare April 7, 2019 18:52
@msvbg
Copy link
Contributor Author

msvbg commented Apr 12, 2019

I am happy to announce that this now passes CI, after using esy 0.5.6.

@OhadRau
Copy link
Collaborator

OhadRau commented Dec 27, 2019

Whoops, looks like this PR kinda got ignored 😅 Hopefully the work for this is superseded by #685 but gonna keep this open temporarily in-case a similar bug exists in new text-wrapping lib since it's using String.sub as well.

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