Claude/update jsonic library gj8 zb#4
Conversation
- Build a declarative GrammarSpec encoding open-rule mods, close-rule
mods and the directive rule's own alts
- Apply via jsonic.grammar(spec, { rule: { alt: { g: 'directive' } } })
so every generated alt gets 'directive' appended to its group tags
- Keep jsonic.rule(name, ...) solely for rs.clear()/bo/bc hooks which
aren't expressible in a GrammarSpec
- All 9 tests pass
- Build a declarative GrammarSpec for all rule modifications plus the
directive rule's own alts
- Wire bo/bc state actions via auto-resolved FuncRefs (@<name>-bo,
@<name>-bc) registered in the GrammarSpec.Ref map
- Register per-rule condition closures as AltCond FuncRefs to support
rulemod C predicates in open/close grammar alts
- Apply via j.Grammar(gs, &GrammarSetting{Rule:{Alt:{G:"directive"}}})
so every alt auto-gets the 'directive' group tag
- Resolve shared close tokens by looking up the existing TinName so
the grammar spec references the correct registered token
- All 5 Go tests pass
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10cb72e471
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Object.entries(rules.close).forEach((entry: any[]) => { | ||
| const [rulename, rulespec] = entry | ||
| const r = ruleFor(rulename) | ||
| r.close = [ |
There was a problem hiding this comment.
This assignment overwrites any close alternatives already collected for the same rule in the rules.open pass. If a user config puts the same rule in both rules.open and rules.close, the earlier fallback [{s:[CLOSE], b:1, g:'end'}] alt is dropped, which changes behavior from the previous implementation where both rs.close(...) additions coexisted. That regression can break custom directives that rely on overlapping rule sets (especially when rules.close[r].c is restrictive), so r.close needs to be merged instead of replaced.
Useful? React with 👍 / 👎.
| rs.PrependClose(closeAlt, commaCloseAlt) | ||
| }) | ||
| r := ruleFor(rn) | ||
| r.Close = closeAlts |
There was a problem hiding this comment.
Setting r.Close here replaces the close alt inserted earlier for the same rule in the open-rules loop. In the pre-refactor Go code, both were added via PrependClose, so overlapping Open/Close rule configs retained all close paths; now one set is silently discarded. For custom configurations where a rule is present in both maps, this can prevent valid directive termination paths from being installed.
Useful? React with 👍 / 👎.
2 participants
No description provided.