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 directive keywords to error message for parsing global item #6723

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

e-hat
Copy link
Contributor

@e-hat e-hat commented Dec 13, 2024

Notify the user that they can also use diagnostic, enable, or requires in this context.

Connections
Addresses #6421.

Description
Support for directives has recently improved, so this error message now reflects that the user can also use directive keywords at the top-level.

Testing
Verified locally, did not add a test for this small change in accordance with the style of the tests in this module.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@e-hat e-hat requested a review from a team as a code owner December 13, 2024 20:30
@e-hat e-hat force-pushed the 6421-add-directive-keywords-to-global-item-parse-message branch from 6ca2422 to 40705ca Compare December 13, 2024 20:34
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

LGTM, minus one issue.

@ErichDonGubler ErichDonGubler self-assigned this Dec 16, 2024
@ErichDonGubler ErichDonGubler added type: enhancement New feature or request naga Shader Translator area: naga front-end lang: WGSL WebGPU Shading Language kind: diagnostics Error message should be better labels Dec 16, 2024
@ErichDonGubler
Copy link
Member

Interestingly, the term "item" (from before this PR) is a Naga concept, rather than a standard WGSL one. 🤔 I had hoped to align the vocabulary at the same time as the issue was handled, but I notice that didn't communicate that. I'm happy to save that for a further iteration, or even drop the thought entirely; I don't think it's going to be a problem in practice.

e-hat and others added 3 commits December 16, 2024 15:46
Notify the user that they can also use `diagnostic`, `enable`, or
`requires` in this context.
@ErichDonGubler ErichDonGubler force-pushed the 6421-add-directive-keywords-to-global-item-parse-message branch from e05096a to 68ddfd0 Compare December 16, 2024 20:46
@ErichDonGubler ErichDonGubler enabled auto-merge (squash) December 16, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga front-end kind: diagnostics Error message should be better lang: WGSL WebGPU Shading Language naga Shader Translator type: enhancement New feature or request
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants