Two Distinct Values Assigned to Same Parameter

[wealdtech]

When parsing the RRULE recurrence there is a part in the grammar that says:

recur_rule_part : k_freq ASSIGN freq | k_until ASSIGN enddate | k_count ASSIGN digits | k_interval ASSIGN digits | k_bysecond ASSIGN byseclist | k_byminute ASSIGN byminlist | k_byhour ASSIGN byhrlist | k_byday ASSIGN bywdaylist | k_bymonthday ASSIGN bymodaylist | k_byyearday ASSIGN byyrdaylist | k_byweekno ASSIGN bywknolist | k_bymonth ASSIGN bymolist | k_bysetpos ASSIGN bysplist | k_wkst ASSIGN weekday ;

however in here both 'count' and 'interval' are assigned to 'digits'. It is possible to have both 'count' and 'interval' in the same rule, in which case we lose information.

[bkiers]

Hi,

Yes, good catch.

This ambiguity can easily be resolved by ANTLR4's so called "tree-labels". Checkout this Q&A on stackoverflow to see what "tree-labels" are, and how they can be used: http://stackoverflow.com/questions/14565794/antlr-4-tree-inject-rewrite-operator

The ICalendar grammar I presented here was only a (more or less) 1-on-1 translation of the official RFC. Besides what you just pointed out, I'm pretty sure there are more things that could be improved.

For the time being, I'll leave this issue open, maybe I'll improve the grammar with these "tree-labels" in the near future.

Cheers,

Bart.

[wealdtech]

Interesting. Given that this just seems to be a case of the spec not bothering to define interval and count separately, wouldn't it be easier to do something like:

recur_rule_part : k_freq ASSIGN freq | k_until ASSIGN enddate | k_count ASSIGN count | k_interval ASSIGN interval | k_bysecond ASSIGN byseclist | k_byminute ASSIGN byminlist | k_byhour ASSIGN byhrlist | k_byday ASSIGN bywdaylist | k_bymonthday ASSIGN bymodaylist | k_byyearday ASSIGN byyrdaylist | k_byweekno ASSIGN bywknolist | k_bymonth ASSIGN bymolist | k_bysetpos ASSIGN bysplist | k_wkst ASSIGN weekday ;

count : digits ;

interval : digits ;

Or is there some reason to not do it this way?

[bkiers]

Yes, that is also a valid option.

[wealdtech]

I've altered my fork of this to handle this situation, pull request should be attached if you so desire.