-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[truncate explanation] Fix edge case when we truncate due to max_chars #14105
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
[truncate explanation] Fix edge case when we truncate due to max_chars #14105
Conversation
Previousely the added test case would output: "...Full output truncated (0 lines hidden), use '-vv' to show"
bluetech
left a comment
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.
Thanks! Please see my comments.
sum(len(x for s in strings) is consistentely faster than len(''.join(strings)), see https://claude.ai/public/artifacts/6a4c33e7-9ad5-4078-8ee7-e343984ce087
159b275 to
49bb987
Compare
|
Drafting because I realized that there's a lot of optimization that can still be done. |
Better check the theory and the len of the string to know if we're going to truncate rather than checking the full length of the result after the fact.
bluetech
left a comment
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.
Thanks for splitting, looks good, thanks!
Previousely the added test case would output:
"...Full output truncated (0 lines hidden), use '-vv' to show"
Also removed some useless complexity.