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 for #3630 #3631

Merged
merged 2 commits into from
Jul 29, 2023
Merged

Conversation

pasto9
Copy link
Contributor

@pasto9 pasto9 commented Jul 26, 2023

I made the scripts more pythonic, used contextmanagers for the file io and used regEx to for finding and replacing the strings.

While working on it I tried to stick to pylints conventions for coding styles.

This is my first pr on a public repo. So please let me know if I did anything wrong.

@KvanTTT KvanTTT added the cpp label Jul 26, 2023
from glob import glob
from pathlib import Path

def main(argv):
for file in glob("./parser/*.g4"):
Copy link
Contributor

@kaby76 kaby76 Jul 26, 2023

Choose a reason for hiding this comment

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

The "parser/" directory is required because he Antlr tool generates code with a "package parser" declaration. And, there's no way to stop the Antlr tool from doing that. Either include "parser/" in the string passed to glob, or maybe just recurse/glob "*.g4"?

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I'm hoping all this "transformGrammar.py" hack can go away with antlr/antlr4#4345. But, who knows if it will be added.

@pasto9
Copy link
Contributor Author

pasto9 commented Jul 27, 2023

I added the "parser/"directory in the string passed to glob.

If it goes away. Then that is the way it is. I just wanted to contribute something.

@teverett
Copy link
Member

@pasto9 thanks!

@teverett teverett merged commit 15e59da into antlr:master Jul 29, 2023
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants