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

Add --error-format flag to serialize err diagnostics #1740

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

yannham
Copy link
Member

@yannham yannham commented Dec 18, 2023

Closes #1738.

This PR adds an --error-format global flag (copied over from Rust, after benchmarking what a few other compilers are donig, this seemed to be the better tradeoff between being intuitive and not conflicting with other meaning of "format as JSON", which we have a lot in Nickel - export format, input format, etc.).

The user can choose between JSON, TOML or YAML - I believe JSON is the most common one, but it's virtually free to support more formats with serde (at the least the format that we export Nickel configurations to), hence I don't see an obvious reason to not support them.

Incidentally, this PR formats more CLI errors using codespan diagnostics (they were just printed as text before), so that the flag really affects all error messages. As a bonus, those error messages are now nicely colored like the others.

@github-actions github-actions bot temporarily deployed to pull request December 18, 2023 18:22 Inactive
@github-actions github-actions bot temporarily deployed to pull request December 19, 2023 14:07 Inactive
@yannham yannham marked this pull request as ready for review December 19, 2023 14:10
Copy link
Member

@jneem jneem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this imply any stability guarantees for the error messages? I'd imagine that if people are consuming them programmatically then they're expecting at least part of the format to be stable...

@yannham yannham added this pull request to the merge queue Dec 19, 2023
Merged via the queue into master with commit d35baae Dec 19, 2023
5 checks passed
@yannham yannham deleted the feat/error-messages-json branch December 19, 2023 15:41
@yannham
Copy link
Member Author

yannham commented Dec 19, 2023

Does this imply any stability guarantees for the error messages? I'd imagine that if people are consuming them programmatically then they're expecting at least part of the format to be stable...

Good question. I would be tempted to say "no", because it makes our life simpler 😛 and also because currently it's just serializing the library datatype that we happen to use (codespan_reporting). It's probably not too hard to convert it if we ever change the error reporting infrastructure, but still. In the meantime, I should probably add a mention in the documentation of the CLI argument that the format isn't stable.

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.

Serialize errors as JSON
2 participants