-
Notifications
You must be signed in to change notification settings - Fork 5
refactor: remove rules #24
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
refactor: remove rules #24
Conversation
Agustindeleon
commented
Feb 28, 2025
- Separate "remove" rules to generate independent events at the listener level
- Change int_range from a token to a parser rule in order to use the start and the end of range as integers
Looks good, haven't tested it myself. |
Actually, could you please add a test for the modified functionality @Agustindeleon? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add test for changed functionality
Actually, there are some simple test cases for these kinds of directives that ensure that the lexer and parser don't throw errors during their execution. |
I know. But "no error" is not the same as "produces the correct output". The test coverage is still too low for my taste, and your change is a great chance to improve test coverage. The new test would also document the expected behaviour of the parser. |
@Agustindeleon Can you add more tests somehow? |
…elar/seclang_parser into refactor/removeRules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff!