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

REPL: make truncation by characters configurable #16167

Merged

Conversation

mpollmeier
Copy link
Contributor

via -Vrepl-max-print-characters

The default character limit changes: instead of cutting at 1000 chars, it is now 50k. The previous default was rather low, and only made sense because it was also (and confusingly) used for the MaxElements truncation.

This compliments #16011 and is based on our discussion there, specifically #16011 (review)

I dropped the i11377 test because it didn't seem to add any value over the new test, while being quite confusing to read. I can add it back though...

via `-Vrepl-max-print-characters`
The default character limit changes: instead of cutting at 1000 chars,
it is now 50k. The previous default was rather low, and only
made sense because it was also (and confusingly) used for the
`MaxElements` truncation.

This compliments scala#16011 and is
based on our discussion there, specifically scala#16011 (review)
@prolativ
Copy link
Contributor

It would be nice to have a test showing the interaction between truncation on elements and truncation on characters limit. Especially, might the latter setting truncate the truncation message from the former, something like 1234 ... large output trun ... large output truncated? It would be nice if we could avoid such behaviour.

@mpollmeier
Copy link
Contributor Author

That's a good idea, thanks, will do!

@mpollmeier mpollmeier force-pushed the repl/configurable-truncation-character-limit branch 2 times, most recently from 2e507f8 to 1475a6f Compare October 16, 2022 17:28
@prolativ
Copy link
Contributor

@mpollmeier are you sure you committed all your changes? I can't see any difference

@mpollmeier
Copy link
Contributor Author

I didn't do anything yet, aside from rebasing the branch, which confused the github PR web view, so I reverted to the old version before the rebase.

Will let you know when it's ready to re-review.

@mpollmeier
Copy link
Contributor Author

It's ready to re-review now.
The truncation is unified in one place and I added a test to ensure we're not appending the info-text twice.

@prolativ prolativ merged commit b88aa12 into scala:main Oct 17, 2022
@Kordyjan Kordyjan added this to the 3.2.2 milestone Aug 1, 2023
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.

3 participants