_find_width_index and _handle_long_word change #2
Conversation
xi
left a comment
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.
| # 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)There was a problem hiding this comment.
If performance is an issue, a binary search might be an option.
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).
2 participants
(the previous PR was made with the incorrect account, apologies)
_find_width_index and _handle_long_word change
See python#28136 (specifically this comment)