Check syntax #103
Check syntax #103
Conversation
rivescript/parser.py
Outdated
| total = parens + square + curly + angle # At each character, not more than 1 bracket opens | ||
| for special_char_count in [parens, square, curly, angle, total]: | ||
| if special_char_count not in (0, 1): | ||
| return "Unbalanced brackets" |
There was a problem hiding this comment.
Would this raise syntax errors on a trigger like + my name is (<bot name>|<bot master>)? It seems to check that there is at most one active bracket open, per character scanned (the checks a couple lines further down check the counts after the whole trigger has been scanned, to check if there are any dangling mismatched brackets at the end).
Another example trigger that'd be plausibly in a bot would be an angled tag inside an optional, like idk,
+ [<bot name>] *
- You called? <@>
I think the better fix for #86 is to move the syntax check call to after the ^Continue is handled, around this part of the code. Here, we're calling check_syntax near the top and then looking ahead after that. (This would mean the bug also extends to any command with strict syntax rules, like if you continued a ! Definition or *Condition in the wrong spots)
For fixing the syntax checker itself, it would be ideal if it could detect things like "an opening ( with a closing ] somewhere inside it", like + [ some ( invalid ] ordering ), which raise a regexp syntax error but my code doesn't catch it because it's all balanced. But I don't know how the code could check that while not looking like a tangled mess.
There was a problem hiding this comment.
I actually wasn't aware of that usage of angle brackets. Does {} only apply to e.g. {weight=100}? Or I can assume that curly brackets would not be nested in other brackets ([] ()). If so, I will simply remove angle from the total, which means that [, ( { can only open one at a time.
There was a problem hiding this comment.
Yeah the only curly bracket tag allowed in triggers is {weight}, and it doesn't make sense to embed anything inside it (the regexp that parses the {weight} tag only expects to find a number in there).
So your idea sounds good -- while doing things like ((hello|hi) (robot|computer)) are allowed in regexps, I don't condone that kind of syntax in RiveScript and none of the docs mention it, so I would like it to become a syntax error. 😄
There was a problem hiding this comment.
Ok I will update the merge request later accordingly.
|
@kirsle I updated the check along with unit tests. |
Provide stricter check for syntax that slipped through earlier check. Following up from the comment in #86.