simonpoole / OpeningHoursFragment

Android UI element for displaying and editing an opening hours value
MIT License
9 stars 2 forks source link

opening_hours rule separator lacking space #23

Closed rjw62 closed 6 years ago

rjw62 commented 6 years ago

When using the new opening_hours interface to create or edit and opening_hours value, multiple rules entered are separated by a semi-colon with no spaces either side. But according to https://wiki.openstreetmap.org/wiki/Key:opening_hours/specification the is "; " -- i.e. a semi-colon followed by a space.

(The opening_hours interface does seem to be able to correctly parse existing entries that use "; " as the rule separator. Also, it correctly uses "," (without a space) to separate time ranges in and days in etc.)

The behaviour above noted with Vespucci 10.2.0 on Android, downloaded through Google Play.

simonpoole commented 6 years ago

I actually consider the requirement for a space a bug in the specification in particular as there is no real reason why it should be needed (it does make reading the values a bit easier). The parser simply ignores white space where possible, so that's not an issue.

@ypid any reason for this from your side?

rjw62 commented 6 years ago

I always presumed it was a deliberate decision in the spec, precisely to improve human readability. It also allows slightly more sane line breaks if the (sometimes long) value strings need to be split over multiple lines. Even if the space is a bug in the spec, the spec is unambiguous and the space is commonly used in tagged objects (at least with opening_hours for Post Offices and collection_times for Post Boxes that I run tools for). So I think it would be better for Vespucci to follow the spec when writing time ranges unless/until the spec gets changed.

ypid commented 6 years ago

I agree with @rjw62 last comment. There is not much to add from my side. The space was already common as I started looking into opening_hours in 2013 and I kept it as is. I appreciate the human readability although it is not really required. Interestingly, in the original spec by Netzwolf did not contain the space. I considered it a bug in the spec by Netzwolf and fixed it in revision 1076196.

Funny side note: @simonpoole you asked in the spec talk page for the use of spaces in the syntax to be clarified. Ref: https://wiki.openstreetmap.org/wiki/Talk:Key:opening_hours/specification

simonpoole commented 6 years ago

It is one thing suggesting that for pretty printing a space should added (no problem with that), but making white space on input a requirement would seem to be bad. In a strict parsing mode this would cause errors to be thrown which would definitely raise some eyebrows.

In particular it seems to be completely arbitrary as other tokens that are used as separators/punctuation don't have the requirement either, in particular - isn't <space>-<space> (or for other uses of ,).

simonpoole commented 6 years ago

Addressed in https://github.com/simonpoole/OpeningHoursParser/commit/b8422dfcca820004c209d98c9a6796785a03f93c