-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
consider lifecycle prefixes in osmTagSuggestingArea #8881
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea!
@@ -210,7 +199,7 @@ export function rendererFeatures(context) { | |||
|
|||
for (var i = 0; i < strings.length; i++) { | |||
var s = strings[i]; | |||
if (past_futures[s] || past_futures[tags[s]]) { return true; } | |||
if (osmLifecyclePrefixes[s] || osmLifecyclePrefixes[tags[s]]) return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense that intermittent:*
would be filtered out by the past/future filter? (Also not:*
, discussed above.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what the right answer is for intermittent:*
is, but bear in mind that intermittent:*
is only used once in the world (cf. other prefixes like disused:*
are used >200,000 times). It seems like intermittent=yes
is the more common tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the intermittent
entry originated from 494e247. It was probably intended to support intermittent=yes
, not intermittent:*=*
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding that. I guess it would be logical for the past/future filter to match all the lifecycle prefixes except intermittent
Ways with lifecycle prefixes as always rendered as a line in iD, even if the feature would normally be treated as an area (for example
disused:amenity=school
orabandoned:leisure=stadium
).This PR makes iD consider lifecycle prefixes when deciding whether a way is a line or an area.
Example for
proposed:building=yes
: