Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

description related to tag starting with name is removed #10521

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

description related to tag starting with name is removed #10521

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6866012
description related to tag starting with name is removed
draunger Nov 1, 2024
3ee7028
whitspaces are removed
draunger Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions modules/services/taginfo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export default {
_inflight = {};
_taginfoCache = {};
_popularKeys = {
// manually exclude some keys – #5377, #7485
// manually exclude some keys – #5377, #7485
postal_code: true,
full_name: true,
loc_name: true,
Expand Down Expand Up @@ -222,7 +222,7 @@ export default {
this.keys(params, function(err, data) {
if (err) return;
data.forEach(function(d) {
if (d.value === 'opening_hours') return; // exception
if (d.value === 'opening_hours') return; //exception
_popularKeys[d.value] = true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering how this service excludes name, since it isn’t explicitly listed in _popularKeys. It turns out that name happens to be one of the 100 most popular keys that we add to the _popularKeys here. (There’s some additional explanation at #7485 (comment) that I had forgotten.)

});
});
Expand Down Expand Up @@ -289,7 +289,7 @@ export default {
values: function(params, callback) {
// Exclude popular keys from values lookups.. see #3955
var key = params.key;
if (key && _popularKeys[key]) {
if (key && _popularKeys[key] || key.slice(0,4)==='name') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String.prototype.startsWith is less error-prone than comparing a substring manually. A more thorough fix would catch any key if it’s just a localized version of any of the popular keys. A localized key looks like baseKey:xy, where xy is a language code (not necessarily two characters long). I think what you’ve got is OK, but I’d recommend switching to startsWith and also requiring a colon, like name:, in case someone types in a key that doesn’t have it like name_disaster or name_base.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@1ec5 as you said that start with name: will cause to name that starting without colon shows suggestion, so do i change with this key.slice(0,4)==='name' to this => key.slice(0,5)==='name:' ??

callback(null, []);
return;
}
Expand Down