Modify

Opened 6 years ago

Closed 6 years ago

#11498 closed enhancement (fixed)

[Patch] Warn about obvious misspelled tag values

Reported by: mdk Owned by: team
Priority: normal Milestone: 15.05
Component: Core validator Version: latest
Keywords: Cc:

Description

As I outlined in #11433 there would be a nice validator enhancement to detect common typos in tag values compared with the values from the presets:

  • using of upper case characters
  • using of ' ' or '-' instead of '_'
  • trailing or leading special chars like ' ', ';', '_'

This extension is very restrictive. Only in case of a validation result "Preset do not contain property value" there would be an additional check for a similar value in the preset. Only if the original value was not found in preset, but the "prettified" value exists, a warning "Misspelled property value" will be shown instead (with fix).

Attachments (2)

patch.diff (9.2 KB) - added by mdk 6 years ago.
patch2.diff (1.8 KB) - added by mdk 6 years ago.
patch extension

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by mdk

Attachment: patch.diff added

comment:1 Changed 6 years ago by Klumbumbus

Component: CoreCore validator

comment:2 Changed 6 years ago by Don-vip

Milestone: 15.05

comment:3 Changed 6 years ago by mdk

Nice that you will fit the patch into the may release.

I saw a little flaw in my patch: in prettifyValue() i do the character replacement twice.

In the first try I used commens.lang.StringUtils, but I saw that this package is not used in JOSM, so I extend Utils.strip(). I hope this is the correct way.

Is it correct, that I need to implement the check twice if I want to provide a fix? First in check(), where I add a message to errors. This would be also a nice place to create the fix Command. But instead I had to ensure that isFixable() returns true on this TestError and fixError() must return the the fix Command. For this I have to implement the check a second time to find out when to create the fix command (there are several reasons for isFixable()=true) and especially the details for the fix command.

Wouldn't it be better to create the fix command in check() and add it to the TestError? isFixable(TestError) just have to look if TestError contains a fix command. And getting the correct fix command would also be extreamly simple!

comment:4 Changed 6 years ago by Don-vip

Resolution: fixed
Status: newclosed

In 8435/josm:

fix #11498 - Warn about obvious misspelled tag values (modified patch by mdk) + javadoc

comment:5 in reply to:  3 Changed 6 years ago by Don-vip

Replying to mdk:

Nice that you will fit the patch into the may release.

Thanks for this nice patch :)

I saw a little flaw in my patch: in prettifyValue() i do the character replacement twice.

Thanks, I have fixed it in the commit.

In the first try I used commens.lang.StringUtils, but I saw that this package is not used in JOSM, so I extend Utils.strip(). I hope this is the correct way.

Yes, perfect.

Is it correct, that I need to implement the check twice if I want to provide a fix? First in check(), where I add a message to errors. This would be also a nice place to create the fix Command. But instead I had to ensure that isFixable() returns true on this TestError and fixError() must return the the fix Command. For this I have to implement the check a second time to find out when to create the fix command (there are several reasons for isFixable()=true) and especially the details for the fix command.

Correct but not optimal, see below.

Wouldn't it be better to create the fix command in check() and add it to the TestError? isFixable(TestError) just have to look if TestError contains a fix command. And getting the correct fix command would also be extreamly simple!

Definitively :) That's why the FixableTestError is here, I have used it in the commit.

comment:6 Changed 6 years ago by mdk

Thank you very much.

Knowing the object model would derfinitly save time :)

With the FixableTestError I think I will extend the test - for the first try the missing test was too complex.

So I'll be back ;-)

comment:7 Changed 6 years ago by mdk

Also outlined in #11433:

Additionally we could also have a test where we remove the '_' from the preset value. If we find a match in this case like "steakhouse" would match "steak_house" without '_', we can warn and offer a fix to replace cuisine=steakhouse with cuisine=steak_house.

I have implemented this extension too.

Changed 6 years ago by mdk

Attachment: patch2.diff added

patch extension

comment:8 Changed 6 years ago by mdk

And the next step could be external configured pais with value typos to be added to the HashMap.

To increase performance these HashMaps could be cached insted of created for each check.

And we could do similar checks for keys.

comment:9 Changed 6 years ago by holgermappt

Please correct file core/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java line 425 ({2}}):

String i = marktr("Value ''{0}'' for key ''{1}'' looks like ''{2}}.");

comment:10 Changed 6 years ago by Don-vip

Resolution: fixed
Status: closedreopened

comment:11 Changed 6 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 8438/josm:

fix #11498 - Warn about obvious misspelled tag values (modified patch by mdk) + i18n fix

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.