Using grammar parser for variables#97
Open
Krishn1412 wants to merge 13 commits into
Open
Conversation
Added new grammar logic for variable. Replaced the regex with Lark implementation.
Run CI on PR for main too
Owner
|
Could you add some testing in https://github.com/tbarbette/npf/blob/main/integration/test_unittest.py? Also can you rebase so the tests will run? Thanks! |
Added new grammar logic for variable. Replaced the regex with Lark implementation.
Added unit tests and rebased the PR.
Owner
|
The tests do not seem to actually test the parsing, does it? |
Author
|
The test checks for the substitution which involves parsing. Should I add tests just for the parsing? |
Added new grammar logic for variable. Replaced the regex with Lark implementation.
Added unit tests and rebased the PR.
Also merge tbarbette#97 Advances on tbarbette#96
Owner
|
Hi! I pushed a rebase on PRtests and highlighted a problematic case. Could you look into it? Basically $(( )) should not be replaced as it is evaluated using asteval. However, what's inside it should be parsed. Please rebase / start again on top of PRtests. Thanks |
I have rebased from the PRTest branch and updated the code to handle the $((..)) case. I also found another issue, "Hello $foo.", was not being parsed. I have fixed this too. In the test cases you have added, for test_complex_01, should echo be removed after parsing?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added new grammar logic for variable.
Replaced the regex with Lark implementation.