_find_width_index and _handle_long_word change - #2
xi merged 2 commits into
Conversation
Sorry, something went wrong.
xi
left a comment
There was a problem hiding this comment.
Great find and great solution! There are just some minor issues that need to be ironed out.
Sorry, something went wrong.
| # for each character | ||
| if self.text_len(text[:width]) == width: | ||
| # For character widths greater than one, width can be more than the number of characters | ||
| return min(width, len(text)) |
There was a problem hiding this comment.
I am not sure if this heuristic works. What would happen in the following case:
lengths = {
'A': 2,
'B': 0,
}
def text_len(s):
return sum(lengths.get(c, 1) for c in s)
wrap('AAABBB', 3)
Sorry, something went wrong.
There was a problem hiding this comment.
If performance is an issue, a binary search might be an option.
Sorry, something went wrong.
There was a problem hiding this comment.
In that case the width of text[:width] (=['AAA']) is not equal to width (=3), so it will calculate it per character. However, if you'd used len as your text_len, you'd get that the width of ['AAA'] = 3, which is correct to it skips the calculation.
Binary search might work, but it'd require disallowing negative character lengths (which I agree don't make sense but the results of binary search might make the results more different than what you'd expect).
Sorry, something went wrong.
(the previous PR was made with the incorrect account, apologies)
_find_width_index and _handle_long_word change
See python#28136 (specifically this comment)