Terms preset does not work properly

[bagage]

If I understood it correctly, terms list in preset are synonyms for one kind of structure. For instance, given the French presets, Digue should be equivalent of Terre-plein (for key embankment). However if a select a line and type Digue, Terre-plein is the 32nd of 34 total results while it should have been the 1st.
The same goes for other words (WC is not leading to toilets, etc.), so am I misinterpreting these terms or is something wrong?

Using iceweasel 40.0 on Debian 4.1.5-1, same behavior on 2 different computers as well as on Windows/Firefox. French installed on both computers but one has iceweasel in French while the other has it in English.

[ghost]

Same here.

Steps to reproduce

1. Set your OSM account to Dutch (nl)
2. Open the iD instance at osm.org
3. Add a point
4. Search for wc

Expected result

Toiletten is the top match.

Actual result

Toiletten isn't even in the list at all.

This seems to work fine for longer keys though: e.g. pinnen correctly resolves to Bankautomaat.

Note

The translation of terms that is included in this instance of iD is WC. I've just added more synonyms in Transifex.

[bagage]

OK, I think I found the reason behind that semi-random behavior. terms array must be filled with lowercase AND comma-separated (without space) synonyms.

For instance, amenity/toilets for French is:

"terms": "Toilettes, WC"

To work properly, it must be changed to:

"terms": "toilettes,wc"

I am not sure where the fix should be though. Some solutions could be:

1. preset.terms should trim terms after splitting?
2. collection.search should convert to lowercase everything when comparing 2 strings?
3. format presets.json and all translated JSON so that preset.terms are correctly formatted. Must be done automatically after fetching new translations from Transifex (maybe in Makefile). This solution is not needed if both 1) and 2) are done.

[ghost]

OK, I think I found the reason behind that semi-random behavior. terms array must be filled with lowercase AND comma-separated (without space) synonyms.

No, that should not have any impact. It is true that lowercase and without spaces is preferable, but it is not necessary to have the search work.

[bagage]

What is doing the work for it then? I tried on a local copy and had it working only by changing as mentioned. But I am not js programmer so I believe you. However I could not find the code handling that; can you point me in its direction? :)

[ghost]

I haven't looked at that code myself, I was told so by an iD developer some time ago. IIRC it splits on commas and then calculates the Levenshtein distance. It may be that with the comma, the Levenshtein distance is bigger, but for longer words this does not matter too much?

[ghost]

This seems to be the relevant code: https://github.com/openstreetmap/iD/blob/master/js/id/presets/collection.js#L75, but I am not really familiar with the code.

[bagage]

Well, Levenshtein distance is not the only method used; basically search is documented to do the following. It will compare the given value converted to lowercase:

1. with all presets' name (lowercased)
2. then all presets' terms (not lowercased)
3. then all presets' tags (not lowercased)
4. then levenshtein distance for all presets' name (maybe is it case insensitive, but not sure about that)
5. then levenshtein distance for all presets' terms (maybe is it case insensitive, but not sure about that)
6. then it concatenates all these result in the order above (not sure about that totally).

Levenshtein distance might find expected result sometimes - but not in first position (see my first post with Terre-plein and Digue) while when a given input value is exactly a term value, it should be the first in the results list . And moreover, Levenshtein may not behave correctly to trailing space nor case issues while this should not affect results list.

edit: renaming Digue to digue in fr.json make it appears first.

[ghost]

Observation of the app's behaviour:

1. changing the case does not affect the search results;
2. putting a space in front of your word does, but it still doesn't find our toilets in the Dutch version.

[bagage]

What did you modify? In nl.json there is only one entry in terms which is WC. If you change it to:

            "terms": "wc"

Then recompile the application entirely with make clean && make (otherwise it seems to not be reloaded). Then launch python -m SimpleHTTPServer and try again. It is working for me, I do not see why we would get different behaviors.

[ghost]

Seems you are right. Without capitals it finds it.

[ghost]

I am working on this. I first changed js/id/presets/preset.js # 49:

/*from*/ return preset.t('terms', {'default': ''}).split(',');
/* to */ return preset.t('terms', {'default': ''}).toLowerCase().split(',');

and it worked but then I thought: why make the client do that every time, when we can settle this at make translations time. Now I am learning lodash to accomplish this.