Opened 5 years ago
Last modified 4 years ago
#18140 closed enhancement
Switch to OpeningHoursParser — at Version 16
Reported by: | Don-vip | Owned by: | simon04 |
---|---|---|---|
Priority: | normal | Milestone: | 20.03 |
Component: | Core | Version: | |
Keywords: | opening_hours nashorn sotm19 java15 | Cc: | Don-vip, ypid23, stoecker, SimonPoole |
Description (last modified by )
Oracle is still threatening to remove Nashorn from Java (to make money with their "GraalVM" stuff).
Java will then no longer provide a JavaScript engine, so we won't be able to run our JS libraries anymore (opening_hours and overpass_turbo).
For opening_hours, there's no need to rewrite the lib in Java, as we can simply use Simon's parser:
https://github.com/simonpoole/OpeningHoursParser
Outstanding issues for the migration:
- Support for
collection_times
,service_times
– not yet supported by OpeningHoursParser? - Localized error messages – WIP: Add translation support by simon04 · Pull Request #30 · https://github.com/simonpoole/OpeningHoursParser/pull/30
The auto-fixing for some cases, such asMo-Tue
,Sa-Su 10.00-20.00
,Mo,Tu 04-17
- (Some tests (mostly of garbage) end with an auto-generated JavaCC exception
badtext
→Encountered " <UNEXPECTED_CHAR> "b "" at line 1, column 1.
) - Integrate OpeningHoursParser as JAR (via Ivy) or svn:externals (run JavaCC in build.xml)
Some considerations to take into account for "wd":
[00:40:47] <simonpoole> I really need to have a look at generating more descriptive error messages in my parser and add an evaluator but its a month ot two work I suspect
[00:43:07] <simonpoole> don-vip https://github.com/mozilla/rhino
[00:44:00] <don-vip> I know it :) rhino was the javascript engine shipped with java 6/7 until oracle introduces nashorn in 8
[00:44:44] <don-vip> but we won't ship an entire javascript engine just for this. Will probably have to rewrite the library in pure java, which isn't a bad thing
[00:45:54] <simonpoole> well my parser currently has only one small known issue
[00:46:10] <simonpoole> outside of that in implements the full spec
[00:47:36] <don-vip> simonpoole do you have a link to your parser?
[00:48:03] <simonpoole> https://github.com/simonpoole/OpeningHoursParser
[00:49:25] <don-vip> awesome!
[00:49:41] <don-vip> so no need to rewrite the JS library, you made my day Simon
[00:50:18] <simonpoole> it -is- a bit stricter than ypids in that some of the more abstruse deviations from the spec are not handled and some rarely used stuff wd isn't supported but that would be easy to add
[00:50:51] <don-vip> ok, good to know
[00:51:24] <simonpoole> just to be clear wd is not in the spec
[00:52:08] <don-vip> by wd do you mean weekdays or the literal "wd"?
[00:52:46] <simonpoole> the literal "wd" used as an abbrev for Mo-Fr
[00:52:54] <don-vip> ok
[00:53:15] <don-vip> well it's easy to detect it as an error and suggest Mo-Fr instead
[00:53:15] <simonpoole> the thing is that it is not really clear if weekday definition is actually univerally the same
[00:53:24] <don-vip> ah
[00:53:29] <don-vip> good point
[00:54:39] <simonpoole> so really it should be added to the spec and clearly defined
[00:54:58] <petschge> that might be fun in israel or some muslimic countries
[00:55:07] <simonpoole> exactly
Change History (16)
comment:1 by , 5 years ago
Keywords: | nashorn sotm19 added |
---|
comment:3 by , 5 years ago
Milestone: | → 20.03 |
---|
follow-up: 6 comment:5 by , 5 years ago
Replying to ypid23:
in terms of usability it might be worth considering me hosting an API endpoint that does the validation for JOSM and we move the calls to opening_hours.js out of JOSM into a NodeJS thing I could host. I do similar things already (ref: https://openingh.openstreetmap.de/api).
Can we use this existing https://openingh.openstreetmap.de/api endpoint for that?
comment:6 by , 5 years ago
Replying to Don-vip:
Can we use this existing https://openingh.openstreetmap.de/api endpoint for that?
Not yet. The opening_hours_server.js needs to be extended. Currently, only /api/oh_interpreter
is provided which fetches all opening_hours via Overpass for a given bbox. Here's an example: https://github.com/opening-hours/opening_hours_server.js#example-query
comment:7 by , 5 years ago
Here's a draft implementation: https://github.com/opening-hours/opening_hours_server.js/pull/3
ypid in https://github.com/opening-hours/opening_hours_server.js/pull/3#issuecomment-590117627
Looks good to me, thanks! As you marked it WIP, I just deployed it to openingh.openstreetmap.de for testing but did not merge the PR yet: https://openingh.openstreetmap.de/api/validate?value=PH
comment:9 by , 5 years ago
Cc: | added |
---|
follow-up: 11 comment:10 by , 5 years ago
The patch attachment:18140.patch rewrites the OpeningHourTest to make use of https://github.com/simonpoole/OpeningHoursParser
For testing, compile OpeningHoursParser and put its JAR file in the classpath. The approach looks rather promising. Currently missing / not working in the patch:
- Support for
collection_times
,service_times
– not supported by OpeningHoursParser? - Localized error messages – first step made via https://github.com/simonpoole/OpeningHoursParser/pull/28
- The auto-fixing for some cases, such as
Mo-Tue
,Sa-Su 10.00-20.00
,Mo,Tu 04-17
- Some tests (mostly of garbage) end with an auto-generated JavaCC exception
badtext
→Encountered " <UNEXPECTED_CHAR> "b "" at line 1, column 1.
@team: What do you think? Which way to follow (simonpoole/OpeningHoursParser vs. opening-hours/opening_hours_server.js)?
follow-up: 13 comment:11 by , 5 years ago
Replying to simon04:
- Support for
collection_times
,service_times
– not supported by OpeningHoursParser?
Surprising. Isn't the syntax the same?
@team: What do you think? Which way to follow (simonpoole/OpeningHoursParser vs. opening-hours/opening_hours_server.js)?
OpeningHoursParser first. We need to keep an offline validator, and Nashorn is going to disappear very soon (in Java 15 almost certainly).
We can see next if there's an interest to provide an additional online feature with opening_hours_server.js.
comment:12 by , 5 years ago
Keywords: | java15 added |
---|
comment:13 by , 5 years ago
Replying to Don-vip:
Surprising. Isn't the syntax the same?
collection_times
allows to specify points in time w/o a range (such as Mo-Fr 17:30
).
comment:14 by , 5 years ago
Add translation support by simon04 · Pull Request #30
· https://github.com/simonpoole/OpeningHoursParser/pull/30
comment:15 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:16 by , 5 years ago
Description: | modified (diff) |
---|
Hey guys, that is quite a situation with Oracle. I have not played with https://github.com/simonpoole/OpeningHoursParser myself yet but I also see it as a viable option as opening_hours validator. I agree with the concerns from simonpoole about the error messages. I see this as really one of the strong parts of opening_hours.js. I invested days where I mapped/corrected real live opening_hours values and for every single value, I made sure that the library gives meaningful/helpful warnings/errors and help how to correct it. It can even correct most errors for you. All of that is used by JOSM for a long time. I guess when we switch to OpeningHoursParser, users would start to miss those features.
I am not sure what the policies of JOSM are and independent of that I would also not like it, but in terms of usability it might be worth considering me hosting an API endpoint that does the validation for JOSM and we move the calls to opening_hours.js out of JOSM into a NodeJS thing I could host. I do similar things already (ref: https://openingh.openstreetmap.de/api). This could also be made optional if the user does not have Internet or if opting out. I think this would be the best option unless Simon can invest those one-two+ months to get the library up on that area. It also would be an answer to Oracle that we as hackers just find a simple workaround for their business strategy stuff.
Let me know what you think. Also, please feel free to include me in such discussions. I might not have had much time in the last months but I still care deeply about Free Software and OSM.