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: newline before module elements #73

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

KaranAhlawat
Copy link
Contributor

@KaranAhlawat KaranAhlawat commented Jul 24, 2024

Closes #72.

I basically followed the idea you left in the issue, and thus gave module_elem precedence over sequential_expressions. It did take me more time than I'd like to admit to grok what you meant and what I had to do though 😅

Now, I added 2 tests to the corpus for these changes. Both tests are failing, but the output doesn't show any deviation in the tree structure, and I loaded the grammar in my editor, it is working fine, so I'm not sure how to proceed. Here is a screenshot to help.

image

Looking forward to your comments!

@KaranAhlawat
Copy link
Contributor Author

Also, as noted in the comments in the grammar.js file, I think this change does mean that there are now no sequential expressions in a module body now, which I'm not sure how big of a deal that is.

Copy link
Member

@Nsidorenco Nsidorenco left a comment

Choose a reason for hiding this comment

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

Really nice work! It even reduced the compiled parser size by a good bit :)

Once the tests are parsing this should be good to go. Thank you so much!

test/corpus/module.txt Outdated Show resolved Hide resolved
test/corpus/module.txt Outdated Show resolved Hide resolved
grammar.js Outdated Show resolved Hide resolved
Copy link
Member

@Nsidorenco Nsidorenco left a comment

Choose a reason for hiding this comment

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

Looks good 💪🏻

@Nsidorenco Nsidorenco merged commit 331d5c9 into ionide:main Jul 24, 2024
5 checks passed
@KaranAhlawat KaranAhlawat deleted the fix-module-newline branch July 24, 2024 15:06
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.

Error while parsing type definition
2 participants