rephrase warning for role location_hint in restriction relation

[GerdP]

What steps will reproduce the problem?

1. Validate ​https://www.openstreetmap.org/relation/228078

What is the expected result?

No warning

What happens instead?

two warnings for the node with role location_hint:
Role verification problem - Role 'location_hint' unknown in templates 'from/via/to' (1)
Unknown role(1)

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

The 2nd warning comes from TurnrestrictionTest. I'll add a correction to the patch for #17561.

Build-Date:2019-04-06 10:28:00
Revision:14963
Is-Local-Build:true

Identification: JOSM/1.5 (14963 SVN en) Windows 10 64-Bit
OS Build number: Windows 10 Home 1803 (17134)
Memory Usage: 687 MB / 1753 MB (119 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:51784, -Dfile.encoding=UTF-8]
Program arguments: [--debug]
Dataset consistency test: No problems found

Plugins:
+ FastDraw (34949)
+ OpeningHoursEditor (34867)
+ apache-commons (34506)
+ buildings_tools (34904)
+ continuosDownload (82)
+ ejml (34389)
+ geotools (34513)
+ jaxb (34678)
+ jts (34524)
+ o5m (34867)
+ opendata (34911)
+ pbf (34867)
+ poly (34867)
+ reltoolbox (34867)
+ reverter (34961)
+ undelete (34919)
+ utilsplugin2 (34932)

Last errors/warnings:
- W: No configuration settings found.  Using hardcoded default values for all pools.
- W: Region [TMS_BLOCK_v2] Resetting cache

[GerdP]

The attached patch seems to work fine. Is it OK?

[Klumbumbus]

I never used that role. I just looked at ​osmwiki:Relation:restriction and it says "A hint to a renderer as to where might be a good place to position a symbol indicating the restriction." That is pure tagging for the renderer which we should not promote. (And there are only ​172 of these roles in the database.)

[GerdP]

I know the tag since a long time so I guess we should not produce any warning, esp. none with a text like "unknown".
Maybe I can modify the test in RelationChecker.

[Klumbumbus]

We could rephrase "Role 'abc' unknown in templates 'xyz'" to "Role 'abc' not in templates 'xyz'" or similar but the warning should stay.

[GerdP]

The rephrase is a good idea.
I've just uploaded a new patch for #17561 which would just suppress the message. I'll think again about it...

[GerdP]

In 14966/josm:

fix #17561 Confusing error message for turn restriction
fix #17567 rephrase warning for role location_hint in restriction relation

• improve validator messages for restriction relations
• suppress duplicate warnings for wrong roles from RelationChecker when TurnrestrictionTest is enabled
• add unit test for TurnrestrictionTest similar to the one for MultipolygonTest

[Klumbumbus]

Hm, I thought you would reword the massage for all "unknown roles", not just for that one role location_hint, at least that makes more sense to me.

And probably we should reword "templates" to "presets".

[Klumbumbus]

Meanwhile I changed the wiki: ​https://wiki.openstreetmap.org/w/index.php?title=Relation%3Arestriction&action=historysubmit&type=revision&diff=1842205&oldid=1790928 after writing on the discussion page: ​https://wiki.openstreetmap.org/wiki/Talk:Relation:restriction#remove_role_location_hint

[GerdP]

Replying to Klumbumbus:

Hm, I thought you would reword the massage for all "unknown roles", not just for that one role location_hint, at least that makes more sense to me.

I intended to do that but it seems I got it wrong.

And probably we should reword "templates" to "presets".

I agree. Should I change all places where template means preset?

[GerdP]

Maybe we were mislead. The unit test expects e.g. Role 'level_x' is not in templates 'outline/part/ridge/edge/entrance/level_-?\\d+'".
So the word preset would be wrong here, it seems to be about a template for the role. Right?

[GerdP]

Proposed changes

[Klumbumbus]

With "template" the table with possible roles and appropriate object types in the lower part of relation presets is meant. As this is "in the presets" too I guess we can use the word "preset" to not confuse the user and use consistend wording.

[GerdP]

Don't think so. I replaced template by preset here:
Look at the test string Role of relation member does not match expression 'power' in template Power Route"
In my patch this is changed to
Role of relation member does not match template expression 'power' in preset Power Route

But Role 'level_x' is not in presets 'outline/part/ridge/edge/entrance/level_-?\\d+' makes no sense.

[Klumbumbus]

Ah yes, it seems better to keep both words.

[GerdP]

In 14990/josm:

see #17567: improve messages created by RelationChecker

• new error code 1709 for " Type {0} of relation member with role {1} does not match accepted types {2} in preset {3}"
• role type -> role
• replace template by preset where appropriate
• adapt unit test and improve speed

[Klumbumbus]

There are still different texts:

[GerdP]

Hmm, yes, both messages come from TurnRestrictionTest which doesn't know the presets or templates.
I could change both messages to "Unknown role {0} in restriction" or maybe "Unexpected role {0} in restriction".
Alternative is to remove all tests which are also implemented in RelationChecker and keep only those which detect special cases like mixed via node/way.

Thinking about it I'd prefer the 2nd solution.

[GerdP]

.. but we would not produce any message when someone disables the presets. What do you think?

[GerdP]

Sorry for the noise. The messages in TurnRestrictionTest should be preferred because some have a higher severity.
The patch 17567-rephrase.patch changes Unknown to Unexpected for both role and member type (relation in restriction) and removes the special case for location_hint. If I hear no complains I'll commit this tomorrow.

[GerdP]

In 15003/josm:

fix #17567: apply 17567-rephrase.patch

[Klumbumbus]

In 15004/josm:

• see #17567 - make validator warnings more comprehensible
• fix #17630 - warn about building:part=* together with building=*
• fix #17593 - warn about sport=football

[Hb---]

Please check the wording again.

"turn restriction" is a two word term.

[Klumbumbus]

Replying to Hb---:

Please check the wording again.

"turn restriction" is a two word term.

I noticed this, but since turnrestrictions is already used several times in translatable strings I didn't want to throw away all the translations and instead went the way of consistency.

[Hb---]

The translation source currently has 57 times the correct spelling and 9 times the bad spelling. Out of these 9 bad times:

• the commit r14966 brought 1 new bad string ("Superfluous turnrestriction as \"from\" way is oneway")
• the commit r15004 brought 3 new bad strings ("Unknown turnrestriction", "Unexpected role ''{0}'' in turnrestriction", "Unexpected member type in turnrestriction")

Not throwing away existing translations is a good reason to keep bad source strings. But it should be avoided to serve the translators another 4 bad strings introduced with the commits in this bug.

Please correct the new bad strings.

[GerdP]

I agree. In r14966 I also kept the bad spelling without a good reason.

[Klumbumbus]

was fixed in r15019
@Gerd: there was a typo the commit message (missing #) in r15019 therefore it didn't reference and close this ticket automatically. You can modify the commit message for such cases.

[GerdP]

Ah, sorry, I know how to edit the commit message but I did not notice that typo. Do you want me to do it now?

[Klumbumbus]

It's your choice.

[GerdP]

In 15019/josm:

fix #17567: fix spelling turnrestriction -> turn restriction