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

Update error patterns #194

Merged
merged 7 commits into from
Apr 25, 2023
Merged

Update error patterns #194

merged 7 commits into from
Apr 25, 2023

Conversation

swernli
Copy link
Collaborator

@swernli swernli commented Apr 20, 2023

This change updates the error patterns in the compiler to favor only using labels with strings that add more information to the top level error message and make sense when concatenated. It also updates the qsc_wasm error translation to use the label message and help text if present.

@swernli
Copy link
Collaborator Author

swernli commented Apr 22, 2023

The question came up in further discussion having a more specific convention in the error messages. Specifically, treat the top-level message (the thing in #[error("TEXT")]) as the place to put all relevant information, and treat any other label text or help text as purely additive/assistive. In contrast, there are still places even with this change where the top-level message plus the first span message are needed to fully interpret the error. I think the former convention is helpful, because not every consumer/environment is going to have a place to concatenate messages (or know that it should). I'll update these messages to follow the convention and see how they look, then I could use a text review from folks to make sure they all make sense.

This change updates the error patterns in the compiler to favor only using labels with strings that add more information to the top level error message and make sense when concatenated. It also updates the `qsc_wasm` error translation to use the label message and help text if present.

Note this updates the `Ambiguous` error to use a single span by referring to other namespaces by name rather than position.
compiler/qsc_frontend/src/resolve.rs Outdated Show resolved Hide resolved
compiler/qsc_frontend/src/resolve.rs Outdated Show resolved Hide resolved
compiler/qsc_frontend/src/resolve.rs Outdated Show resolved Hide resolved
compiler/qsc_wasm/src/lib.rs Outdated Show resolved Hide resolved
@swernli swernli enabled auto-merge (squash) April 25, 2023 06:20
@swernli swernli merged commit 0bce50e into main Apr 25, 2023
10 checks passed
@swernli swernli deleted the swernli/error-updates branch April 25, 2023 06:21
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.

None yet

4 participants