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

Get rid of shallow_repr and print full terms #1676

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

yannham
Copy link
Member

@yannham yannham commented Oct 11, 2023

For a long time, we've used shallow_repr() as a light pretty printer for terms in error messages. The issue is that this printer is very uninformative: anything past constant is printer as { ... } or [ ... ]. It dates back to times where we didn't have a proper pretty printer, but now we have (and we can cap the output if we don't want the message to be too long).

This PR get rid of shallow_repr and use the Display instance for RichTerm instead. It has the drawbacks of sometimes exposing generated variables (and, very soon, will be closure addresses) - we might want, in the future, to substitute some of the environment variables to get a better output. Still, even with generated variables, we have much more information about the structure of a term in an error message (field names, array length, etc.). Most of the error messages use codespan to print those terms, and codespan is handling long input on its own (only showing the beginning of the snippet and the end). Elsewhere, we used pretty_print_cap.

@github-actions github-actions bot temporarily deployed to pull request October 11, 2023 10:31 Inactive
@yannham yannham requested a review from jneem October 12, 2023 12:58
@yannham yannham added this pull request to the merge queue Oct 12, 2023
Merged via the queue into master with commit 3edcb52 Oct 12, 2023
5 checks passed
@yannham yannham deleted the task/get-rid-of-shallow-repr branch October 12, 2023 17:25
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.

2 participants