search

simonpoole / OpeningHoursParser

Parser for string values according to the OSM opening hours specification
MIT License
32 stars 12 forks source link

Accepts (and prints) invalid additional rules in strict mode #81

Open westnordost opened 9 months ago

westnordost commented 9 months ago

According to the specification https://wiki.openstreetmap.org/wiki/Key:opening_hours/specification#explain:additional_rule_separator , the additional rule separator may only follow after either of a time selector, a rule modifier or a comment.

For example Mo-Th, Jun-Jul Fr is parsed without error in strict mode. It is also printed the same.

The opening hours evaluation tool is unable to deal with this (not even with the lenient parsing it does by default)

There are several avenues how to deal with this:

  1. allow this syntax only in non-strict mode

  2. if such rules are encountered, append open to the rule that would otherwise cause the following rule to be invalid when pretty-printing it , e.g. Mo-Th, Jun-Jul Fr -> Mo-Th open, Jun-Jul Fr

simonpoole commented 9 months ago

Cause: The opening [hours evaluation tool](https://openingh.openstreetmap.de/evaluation_tool/?EXP=Mo-Th%2C%20Jun-Jul%20Fr) is unable to deal with this (not even with the lenient parsing it does by default)

Effect: According to the specification https://wiki.openstreetmap.org/wiki/Key:opening_hours/specification#explain:additional_rule_separator , the additional rule separator may only follow after either of a time selector, a rule modifier or a comment.

We, some exceptions noted, use https://wiki.openstreetmap.org/w/index.php?title=Key:opening_hours/specification&oldid=1075290 as the base line for the parser,

BTW that change to the spec was/is the reason that I actually specify the base version which I based the initial version of the parser on.

westnordost commented 9 months ago

Hm, I see. I don't like that it seems that the spec was changed unilaterally because one's own parser was not able to deal with the syntax. (It's not as if the semantic ambiguity is solved by adding that construction, as Mo-Fr 08:00-18:00, 20:00-22:00 also exists and the spec does not a good job in explaining whether 20:00-22:00 is a new rule or part of the first rule.)

On the other hand, that tool is a bit of the reference implementation, and is not one goal of the pretty-printing to generate a opening hours string in a syntax that will be understood by the largest set of interpreters? E.g. this parser changes TH to Th for this reason, no?

simonpoole commented 9 months ago

That was a good decade ago and subsequentially I was able to convince ypid to do things in a fashion that was more compatible with wider use, for example versioning.

I haven't closed the issue because you do have a point with at least having the facility to produce output that is compatible with later versions.

westnordost commented 9 months ago

FWIW a lot of rules that are invalid as per this restriction actually do have problems, i.e. were simply written erronously. E.g.