Modify

Opened 4 weeks ago

Closed 4 weeks ago

Last modified 4 weeks ago

#17219 closed defect (fixed)

[Patch] TagChecker.isTagInPresets() doesn't work

Reported by: GerdP Owned by: team
Priority: normal Milestone: 19.01
Component: Core validator Version:
Keywords: Cc:

Description

Javadoc says that it returns true "if the given tag is in internal presets"
In fact it returns true if the given tag key is in the presets.

The patched version causes unit test TaginfoTestIT.testCheckPopularTags() to fail as it now finds some values which are not in the presets.

Attachments (2)

17219.patch (628 bytes) - added by GerdP 4 weeks ago.
17219-v2.patch (1.3 KB) - added by GerdP 4 weeks ago.

Download all attachments as: .zip

Change History (11)

Changed 4 weeks ago by GerdP

Attachment: 17219.patch added

comment:1 Changed 4 weeks ago by GerdP

Summary: Checker.isTagInPresets doesn't work[Patch] Checker.isTagInPresets doesn't work

comment:2 Changed 4 weeks ago by stoecker

Hmm, shouldn't probably that values.isEmpty() be an value.isEmpty()?

comment:3 Changed 4 weeks ago by GerdP

Yes, would be an alternative. Anyway, the code requires more changes when this fixed, else we'll see thousands of new errors.
Working on it...

comment:4 Changed 4 weeks ago by Don-vip

I don't think the patch is correct. What happens when calling the method with name=foobar?

comment:5 Changed 4 weeks ago by GerdP

The patched method returns false, which is correct IMHO.

Last edited 4 weeks ago by GerdP (previous) (diff)

comment:6 Changed 4 weeks ago by GerdP

My understanding is this: We have presets where we have a list of values, and we have presets where we only have a key.
So, isTagInPresets(String key, String value) should only return true if the combination of both is in the presets, while
isKeyInPresets(String key) should return true even if no values are present.

Changed 4 weeks ago by GerdP

Attachment: 17219-v2.patch added

comment:7 Changed 4 weeks ago by GerdP

Summary: [Patch] Checker.isTagInPresets doesn't work[Patch] TagChecker.isTagInPresets() doesn't work

v2 of the patch works with the existing unit tests and produces the same results in JOSM validator. If I hear no complains I'll commit it this evening.

comment:8 Changed 4 weeks ago by GerdP

Resolution: fixed
Status: newclosed

In 14704/josm:

fix #17219 TagChecker.isTagInPresets() doesn't work

comment:9 Changed 4 weeks ago by Don-vip

Milestone: 19.01

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
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.