Opened 5 years ago
Closed 5 years ago
#18807 closed defect (othersoftware)
Multiple warnings for one issue with opening_hours
Reported by: | skyper | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Core validator | Version: | |
Keywords: | template_report opening_hours PH | Cc: | ypid23 |
Description (last modified by )
What steps will reproduce the problem?
- Have an object with
opening_hours=Fr-We,PH 10:00-17:00; Fr PH +1 day off
. If Thursday is holiday, Friday is off. - run validator
What is the expected result?
No warning
What happens instead?
Two warnings for one issue.
Opening hours syntax - opening_hours - Fr-We,PH 10:00-17:00; PH +1 day Fr <--- (The selector "weekday" was switched with the selector "holiday" for readability and compatibility reasons.)
Opening hours syntax - opening_hours - Fr-We,PH 10:00-17:00; PH +1 day <--- (The selector "holiday" was switched with the selector "weekday" for readability and compatibility reasons.)
The auto fix changes the value to opening_hours=Fr-We,PH 10:00-17:00; PH +1 day Fr off
which in my understanding is something different but I did not find an example on the wiki.
Please provide any additional information below. Attach a screenshot if possible.
At least two warnings for one issue is a bug
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2020-02-28 00:58:00 +0100 (Fri, 28 Feb 2020) Revision:15951 Build-Date:2020-02-28 02:31:00 URL:https://josm.openstreetmap.de/svn/trunk
Attachments (0)
Change History (12)
comment:1 by , 5 years ago
Keywords: | opening_hours added; opening_hour removed |
---|
comment:2 by , 5 years ago
Cc: | added |
---|
comment:3 by , 5 years ago
Description: | modified (diff) |
---|
comment:4 by , 5 years ago
Description: | modified (diff) |
---|
comment:5 by , 5 years ago
Summary: | Wrong fix off opening_hour → Multiple warnings for one issue with opening_hours |
---|
follow-up: 11 comment:6 by , 5 years ago
The current implementation uses https://github.com/opening-hours/opening_hours.js (othersoftware)
The new implementation via #18140 currently triggers the following error:
Encountered " <HOLIDAYS> "PH "" at line 1, column 26. Was expecting: <EOF>
comment:7 by , 5 years ago
Hi @skyper
Different "selector types" (weekday, holiday) must both match for the rule to match and the rule modifier "off" to apply. The order of the selector types is defined in https://wiki.openstreetmap.org/wiki/Key:opening_hours/specification#section:selector but only there for aesthetics and opening_hours.js and thus currently JOSM supports selector reordering. If selectors are not ordered, you get a warning for every selector. Thus two warnings.
Fr-We,PH 10:00-17:00; PH +1 day Fr off
Looks good to me and should do what you want. You can test the behavior at: https://openingh.openstreetmap.de/evaluation_tool/
Hope this helps.
comment:8 by , 5 years ago
Salut @ypid23
Thanks for the link, overlooked it on the wiki.
Is it possible to have only one warning for both issues?
Opening hours syntax - opening_hours - Fr-We,PH 10:00-17:00; PH +1 day Fr <--- (The selectors "weekday" and "holiday" were switched for readability and compatibility reasons.)
The problem with two warnings is that after fixing the first the second one is obsolete and the auto fix does nothing.
comment:10 by , 5 years ago
Should Fr-We,PH be changed to PH,Fr-We?
I would say the spec is too restrictive here. I updated it. For details refer to: https://wiki.openstreetmap.org/wiki/Talk:Key:opening_hours/specification#Spec_to_restrictive._PH.2CMo-Fr_allowed_but_Mo-Fr.2CPH_is_not.
Is it possible to have only one warning for both issues?
Thanks for explaining. I am open to do that when it actually fixes the issue. I remember that JOSM has a generic issue with warning handling together with opening_hours.js because opening_hours.js can output multiple warnings. Auto fixing one might fix multiple warnings. One idea to fix this could be to put all opening_hours.js warnings into one JOSM warning? Note this issue might also be irrelevant now because of #18140.
comment:11 by , 5 years ago
From JOSM's side this can probably be closed as "other software" as #18140 was fixed.
In general, one summarized message for one tag is much better.
comment:12 by , 5 years ago
Resolution: | → othersoftware |
---|---|
Status: | new → closed |
If still relevant, please report to https://github.com/simonpoole/OpeningHoursParser/issues
Ok, according to osmwiki:Key:opening_hours/specification#weekday_selector the warning seems correct, so only multiple warnings for one issue is the problem.