Fix skipping of new repeated rules

kwasielewski commented 2 months ago

This PR add an extra check to ensure that the rule is skipped only if both lhs and rhs together are already present in the grammar.

Minimal example of a grammar affected by the change:

g = @cfgrammar begin
           S = 1 + A
           S = 2 * B
           A = 1
           B = 1
           B = 2
       end

Previously rule B = 1 would be skipped.

sebdumancic commented 2 months ago

thank you for the fix! this makes sense; it is a case we have overlooked.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 45.95%. Comparing base (ce15a3a) to head (1330d9c). Report is 2 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #86 +/- ## ======================================= Coverage 45.95% 45.95% ======================================= Files 8 8 Lines 457 457 ======================================= Hits 210 210 Misses 247 247 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ReubenJ commented 2 months ago

Makes sense to me, too, @kwasielewski. Thanks for the contribution! Before we merge, can you add a test for this?

Also, I would like to get @THinnerichs' input, as I think we discussed this change at some point and held off on it for some reason. Now I can't remember why.

kwasielewski commented 2 months ago

I added a testset for this change

Herb-AI / HerbGrammar.jl

Fix skipping of new repeated rules #86

Codecov Report