Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the following format #19

Open
fodor0205 opened this issue Aug 30, 2014 · 4 comments
Open

Add support for the following format #19

fodor0205 opened this issue Aug 30, 2014 · 4 comments

Comments

@fodor0205
Copy link

The following format:
Oct-May: Mo-Fr 08:30-16:30; May-Oct: Mo-Fr 08:30-17:30; Sa 09:00-13:00
gives an Unexpected token: ":,timesep" exception in JavaScript.

I've checked it with http://openingh.openstreetmap.de/evaluation_tool/ and it displayed the desired info.

This facility has two seasons:

  • from May 1 to Sept 30:
    • open from Monday to Friday 08:30-17:30
    • open on Saturday 09:00-13:00
  • from Oct 1 to Apr 31:
    • open from Monday to Friday 08:30-16:30

Please correct me if my expression is wrong. Thanks.

@ypid
Copy link
Contributor

ypid commented Aug 30, 2014

Hi

The colon as separator was introduced by Netzwolf. See the original specification from Netzwolf in the wiki back in 2010-2011 (See also Anzeige und Auswertung von "opening_hours" if you speak German). Somehow, @AMDmi3 never agreed with it‘s use in that way. I have had a disscussion with @AMDmi3 about that but currently don‘t find it. You might want to look over the issues on this project.

Note that the evaluation tool is based on a fork of this project (in which I have basically closed all open issues of @AMDmi3 version) and will probably never merged back in here. So I have basically taken over this project and now do my best to maintain it. Adding support for colons was one of the first things I did.

You can express your opening_hours value in a way that is compatible with both versions. Just avoid the colon as selector separator like this:

Oct-May Mo-Fr 08:30-16:30; May-Oct Mo-Fr 08:30-17:30; Sa 09:00-13:00

Note that this values does not express what you wanted to express:

This facility has two seasons:

  • from May 1 to Sept 30:
    • open from Monday to Friday 08:30-17:30
    • open on Saturday 09:00-13:00
  • from Oct 1 to Apr 31:
    • open from Monday to Friday 08:30-16:30

The Sa 09:00-13:00 will apply the whole year. Also, you want "May 1 to Sept 30", but May-Oct does include the whole October also. You would have to rewrite your value like this:

Oct-Apr Mo-Fr 08:30-16:30; May-Sep Mo-Fr 08:30-17:30; May-Sep Sa 09:00-13:00.

You could also express it exactly the way it was specified like this:

May 1-Sep 30 Mo-Fr 08:30-17:30; May 1-Sep 30 Sa 09:00-13:00; Oct 1-Apr 30 Mo-Fr 08:30-16:30. Note that the April does not have 31 days, which the evaluation tool also would have told you 😄).

@Fodi69 May I ask you where you use this library?

@fodor0205
Copy link
Author

Hi @ypid,

I currently develop the hungarian Openstreetmap community website:
www.openstreetmap.hu

I've only heard about the Openstreetmap project half year ago, so I'm relatively new, sorry if I have no relation to osm history.

I basically want to display the opening hours in a human readable format.

I currently display the opening hours of the current week, you can check it out in action at
http://www.openstreetmap.hu/?zoom=17&type=node&id=1348012516
(Nyitvatartás means opening hours, the current day is bold.)

I'm happy to switch to your library, you convinced me and I'm checking the values with your online tool anyway.

Thanks for the clarification of values, I will correct it in OSM.
I think I understand the format a little better now.

Does this forking mean, that there are two competing opening hours formats? That would be bad.

After a google search for a javascript opening hours parser, I found this library, so I think it would be nice to merge the two projects somehow. It would make things a lot cleaner.

@AMDmi3
Copy link
Owner

AMDmi3 commented Sep 4, 2014

I'd like to do it too, but for that @ypid would need to spit his big change into smaller topic bits. It'd be impossible to review it otherwise.

@ypid
Copy link
Contributor

ypid commented Sep 6, 2014

I currently develop the hungarian Openstreetmap community website: www.openstreetmap.hu

Nice nice. You might want to check out https://github.com/dotevo/osm24.

Does this forking mean, that there are two competing opening hours formats? That would be bad.

Kind of. But only in one direction. My version is fully backwards compatible with @AMDmi3 version.

I'd like to do it too, but for that @ypid would need to spit his big change into smaller topic bits. It'd be impossible to review it otherwise.

As said in #14 this would also mean a lot of work for me. And as you don’t have time for this project anyway, I do not see much sense spending hours in splitting up the project in small feature branches. Also, if I would do that that would probably result in the situation that you do not like some features and not merge them and this would even increase the problem of incompatibility. So I would say I leave this as is and continue maintaining my fork of the project.

@AMDmi3 I don’t want to keep you out of the project or something. You are highly encouraged to contribute if you like as you are one of two persons who know the core code of opening_hours.js. All your initial coding and design is still in use by my fork. I am very grateful for the work you have done in the designing phase of this library. I could have never done this. Netzwolf (who wrote a pretty sophisticated opening_hours library needed 3 rewrites or something to get this "right" (which has been withdrawn in favor of opening_hours.js) and you got it with the first one 👍). I could never have gone so far with this project without your initial work.

Please consider to solve #20 to avoid further confusion. If you don‘t want to merge my whole fork, I can offer you to port bug fixes back. Just confirm: See #18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants