-
-
Notifications
You must be signed in to change notification settings - Fork 58
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 crashes when wrapping text containing colorized content #52
Fix crashes when wrapping text containing colorized content #52
Conversation
@@ -315,6 +315,34 @@ test('align option `left`', t => { | |||
`); | |||
}); | |||
|
|||
test('align option (left) does not throw when colorized content > columns', t => { |
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.
this one is not crashing without my fix but I've included it for completeness
test('align option (center) does not throw when colorized content > columns', t => { | ||
const longContent = chalk.green('ab').repeat(process.stdout.columns); | ||
t.notThrows(() => { | ||
boxen(longContent, { | ||
align: 'center' | ||
}); | ||
}); | ||
}); | ||
|
||
test('align option (right) does not throw when colorized content > columns', t => { | ||
const longContent = chalk.green('ab').repeat(process.stdout.columns); | ||
t.notThrows(() => { | ||
boxen(longContent, { | ||
align: 'right' | ||
}); | ||
}); | ||
}); |
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 know that those are not the best tests as they don't quite test the final result but I won't have time to dig into writing better ones.
Thanks for fixing :) |
Without this those repeats:
were called with negative numbers - resulting with:
This was happening because
max
was computed based oncontentWidth
which already handled colorized stuff correctly (thanks tostring-width
being used inwidest-line
) but thelongestLength
only used the.length
property (so it was computed to a greater number):https://github.com/Andarist/boxen/blob/de1f3c89e9c44823cc0c0a26b8b0fb113d648060/index.js#L127