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

feat: underline only the attribute itself in compilation errors about attributes #1524

Closed
wants to merge 3 commits into from

Conversation

i582
Copy link
Contributor

@i582 i582 commented Jan 23, 2025

Not sure if this is a good way to pass location there when we have only SrcInfo

Issue

#1255.

Checklist

  • I have updated CHANGELOG.md
  • I have run all the tests locally and no test failure was reported
  • I have run the linter, formatter and spellchecker
  • I did not do unrelated and/or undiscussed refactorings

@i582 i582 added this to the v1.6.0 milestone Jan 23, 2025
@i582 i582 requested a review from verytactical January 23, 2025 12:55
@i582 i582 requested a review from a team as a code owner January 23, 2025 12:55
@i582 i582 changed the title feat: underline only attribute if error feat: underline only the attribute itself in compilation errors about attributes Jan 23, 2025
src/grammar/next/index.ts Outdated Show resolved Hide resolved
@i582 i582 requested a review from verytactical January 24, 2025 08:22
@@ -946,9 +946,11 @@ const parseConstantDefInModule =
const result = parseConstantDef(node)(ctx);
const firstAttribute = result.attributes[0];
if (typeof firstAttribute !== "undefined") {
// FIXME: should be `firstAttribute.loc`
Copy link
Contributor

Choose a reason for hiding this comment

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

Take a look at this comment please :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll look further into this issue after I'm finished with the current one. It's odd that we have to create a range here.

@i582 i582 closed this Jan 24, 2025
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