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

Fix 3108 #3226

Merged
merged 12 commits into from
Jun 22, 2024
Merged

Fix 3108 #3226

merged 12 commits into from
Jun 22, 2024

Conversation

giacomocavalieri
Copy link
Member

This PR closes #3108

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Nice. Could we emit this warning during parsing rather than storing this information in the AST?

@giacomocavalieri
Copy link
Member Author

Oh yes! I think it will be quite a big change because we don't have a warning emitter during parsing but it's totally doable, I'll change it

@lpil
Copy link
Member

lpil commented Jun 1, 2024

Can we use just a vec? I think I want to remove the warning emitter now that we have an errors vec

@giacomocavalieri
Copy link
Member Author

I'm not sure I understand how that would work, what do you mean by just using a vec?

@lpil
Copy link
Member

lpil commented Jun 2, 2024

As in rather than pass in that emitter class pass in a vec and have it append to the vec.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Lovely! thank you. I've left a few small notes

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
compiler-core/src/parse.rs Outdated Show resolved Hide resolved
compiler-core/src/parse.rs Outdated Show resolved Hide resolved
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Beautiful! Thank you very much!

@lpil lpil merged commit c11ca6b into gleam-lang:main Jun 22, 2024
12 checks passed
@giacomocavalieri giacomocavalieri deleted the fix-3108 branch June 22, 2024 15:59
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.

List pattern matching
2 participants