Modify

Opened 7 months ago

Closed 7 months ago

Last modified 7 months ago

#17684 closed enhancement (fixed)

Confusing error message for wikipedia tag

Reported by: GerdP Owned by: GerdP
Priority: normal Milestone: 19.05
Component: Core validator Version:
Keywords: template_report Cc: Klumbumbus

Description (last modified by Klumbumbus)

What steps will reproduce the problem?

  1. run validator on the attached file

What is the expected result?

a message like "wikipedia tag should not have URL-encoded values like '%27' "

What happens instead?

The error seems to contain the regExp of the rule:
wikipedia=(?i)^[-a-z]{2,12}:.*%[0-9A-F][0-9A-F] tag should not have URL-encoded values like '%27'

Please provide any additional information below. Attach a screenshot if possible.

Found this tagging in an older OSM file (Relation 1163244).

Build-Date:2019-05-06 07:55:22
Revision:15049
Is-Local-Build:true

Identification: JOSM/1.5 (15049 SVN en) Windows 10 64-Bit
OS Build number: Windows 10 Home 1803 (17134)
Memory Usage: 890 MB / 1753 MB (605 MB allocated, but free)
Java version: 1.8.0_191-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
VM arguments: [-agentlib:jdwp=transport=dt_socket,suspend=y,address=localhost:59383, -ea, -Dfile.encoding=UTF-8]
Program arguments: [--debug]
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (34977)
+ apache-commons (34908)
+ buildings_tools (34982)
+ continuosDownload (82)
+ ejml (34908)
+ geotools (34908)
+ jaxb (34908)
+ jts (34908)
+ o5m (34908)
+ opendata (34977)
+ pbf (34908)
+ poly (34991)
+ reverter (34977)
+ undelete (34977)
+ utilsplugin2 (34977)

Validator rules:
+ c:\josm\core\data\validator\geometry.mapcss

Attachments (2)

sample.osm (1.2 KB) - added by GerdP 7 months ago.
17684-fix-java-code.patch (893 bytes) - added by GerdP 7 months ago.
let {0.tag} return the tag of the primitive, not the tag pattern of the selector

Download all attachments as: .zip

Change History (13)

Changed 7 months ago by GerdP

Attachment: sample.osm added

comment:1 Changed 7 months ago by GerdP

The throwError term should probably look like that for wikimedia_commons?

*[wikimedia_commons =~ /%[0-9A-F][0-9A-F]/] {
  throwError: tr("{0} tag should not have URL-encoded values like ''%27''", "{0.key}");
  fixAdd: concat("wikimedia_commons=", trim(replace(URL_decode(tag("wikimedia_commons")), "_", " ")));
  assertMatch: "node wikimedia_commons=File:Foo%27s";
  assertNoMatch: "node wikimedia_commons=File:Foo";
}

*[wikipedia =~ /(?i)^[-a-z]{2,12}:.*%[0-9A-F][0-9A-F]/] {
  throwError: tr("{0} tag should not have URL-encoded values like ''%27''", "{0.tag}");
  fixAdd: concat("wikipedia=", get(regexp_match("(?i)^([-a-z]+:)(.*)$", tag("wikipedia")),1), trim(replace(URL_decode(get(regexp_match("(?i)^([-a-z]+:)(.+)$", tag("wikipedia")),2)), "_", " ")));
  assertMatch: "node wikipedia=en:Foo%27s";
  assertNoMatch: "node wikipedia=en:Foo";
}

comment:2 Changed 7 months ago by GerdP

Cc: Klumbumbus added

I did not commit this change because I don't fully understand why the current rule produces this result. Maybe there is something else that needs to be fixed.

comment:3 Changed 7 months ago by Klumbumbus

{0.tag} doesn't work if it referres to a regexp including (?i)

a workaround is to use *[wikipedia][wikipedia =~ /(?i)^[-a-z]{2,12}:.*%[0-9A-F][0-9A-F]/]

or maybe someone finds a proper fix for this in the josm java code.

(see also ticket:17100#comment:26)

comment:4 Changed 7 months ago by Klumbumbus

Description: modified (diff)

comment:5 Changed 7 months ago by GerdP

Owner: changed from team to GerdP

OK, I'll have a look at the java code.

Changed 7 months ago by GerdP

Attachment: 17684-fix-java-code.patch added

let {0.tag} return the tag of the primitive, not the tag pattern of the selector

comment:6 Changed 7 months ago by GerdP

Milestone: 19.05

I see this also when I remove the (?i). I only found a regex handling for value lists like
[bicycle =~ /^(yes|designated)$/]
where the result for {0.tag} is bicycle=yes|designated instead of bicycle=^(yes|designated)$
I am not experienced with mapcss rules but I guess one would always expect to get the actual tag, not the pattern that matched.

Also, as a normal user I would not want to see such a list in the validator results, as it is still some kind of code instead of the tag that is used.
The attached patch implements this change. If you agree I'll commit it.

For the special case used in this ticket I'd still prefer to see
wikipedia tag should not have URL-encoded values like '%27'
instead of
wikipedia=http://de.wikipedia.org/wiki/Langen_%28bei_Bremerhaven%29 tag should not have URL-encoded values like '%27'

comment:7 in reply to:  6 ; Changed 7 months ago by Klumbumbus

Replying to GerdP:

I see this also when I remove the (?i).

Then it is probably another special character which breaks the output.

Also, as a normal user I would not want to see such a list in the validator results, as it is still some kind of code instead of the tag that is used.

Such lists are used a lot in combinations.mapcss for "x without y" tests e.g. add parking=surface to a node and validate -> "missing tag - parking without amenity=parking|parking_space|parking_entrance|motorcycle_parking". The list is useful as it gives the user assistance which tag to add.

comment:8 in reply to:  7 Changed 7 months ago by GerdP

Replying to Klumbumbus:

Replying to GerdP:

I see this also when I remove the (?i).

Then it is probably another special character which breaks the output.

I don't think so. The code returns the tag key and the regex as val and the special code removes leading ^(and trailing )$

Also, as a normal user I would not want to see such a list in the validator results, as it is still some kind of code instead of the tag that is used.

Such lists are used a lot in combinations.mapcss for "x without y" tests e.g. add parking=surface to a node and validate -> "missing tag - parking without amenity=parking|parking_space|parking_entrance|motorcycle_parking". The list is useful as it gives the user assistance which tag to add.

OK, so you would want those lists but not any other regexp pattern?

comment:9 Changed 7 months ago by Klumbumbus

I read this ticket again from beginning and now I understand your comment:1 🙈. I think all we need to do is just change 0.tag to 0.key at https://josm.openstreetmap.de/browser/josm/trunk/data/validator/wikipedia.mapcss#L50 as there was simply made a typo during wrinting of this mapcss code. 0.tag is not intended in this warning message.

Last edited 7 months ago by Klumbumbus (previous) (diff)

comment:10 Changed 7 months ago by GerdP

Resolution: fixed
Status: newclosed

In 15060/josm:

fix #17684: Confusing error message for wikipedia tag

comment:11 Changed 7 months ago by Klumbumbus

Component: CoreCore validator

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain GerdP.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.