Skip to content

Conversation

@Krishn1412
Copy link

Added new grammar logic for variable.

Replaced the regex with Lark implementation.

Added new grammar logic for variable.

Replaced the regex with Lark implementation.
Run CI on PR for main too
@tbarbette
Copy link
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!

tbarbette and others added 4 commits May 19, 2025 09:57
Added new grammar logic for variable.

Replaced the regex with Lark implementation.
Added unit tests and rebased the PR.
@tbarbette
Copy link
Owner

The tests do not seem to actually test the parsing, does it?

@Krishn1412
Copy link
Author

The test checks for the substitution which involves parsing. Should I add tests just for the parsing?

tbarbette and others added 5 commits June 18, 2025 13:27
Added new grammar logic for variable.

Replaced the regex with Lark implementation.
Added unit tests and rebased the PR.
@tbarbette
Copy link
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?
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.

2 participants