From 5f1360ed0f09581f64a7105015514a3e9d8dc82f Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Fri, 4 Nov 2022 11:45:21 +0100 Subject: [PATCH] don't suggest to "connect the ends" if a feature with area tags matches a line preset For example, when a feature tagged as `highway=primary` (line preset) and `man_made=bridge` (area preset) is mapped as an unclosed way, converting it to an area (by closing the way by connecting the endpoints) does not improve the situation, as then the other tag doesn't fit to the geometry anymore. closes #7037 --- CHANGELOG.md | 1 + modules/osm/tags.js | 5 ++++ modules/presets/index.js | 23 +++++++++++++++- modules/validations/mismatched_geometry.js | 31 +++++++++++++++++----- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dc07bfd2..1b06ba161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ _Breaking developer changes, which may affect downstream projects or sites that * Skip unsupported or invalid turn restriction relations instead of assuming they are a `no_*` restriction ([#9337]) * Fix crash when conflict resolver is opened ([#9345]) * Fix phone number placeholder text now always using the correct localization ([#8380], thanks [@k-yle]) +* Don't don't suggest to "connect the ends" if a feature with area tags also matches a line preset ([#7037]) #### :hourglass: Performance * Speed up start-up by not pre-resolving complex locationSets ([#9347]) #### :rocket: Presets diff --git a/modules/osm/tags.js b/modules/osm/tags.js index 0978e8fb4..101c27fc2 100644 --- a/modules/osm/tags.js +++ b/modules/osm/tags.js @@ -85,6 +85,11 @@ export function osmTagSuggestingArea(tags) { return null; } +export var osmLineTags = {}; +export function osmSetLineTags(value) { + osmLineTags = value; +} + // Tags that indicate a node can be a standalone point // e.g. { amenity: { bar: true, parking: true, ... } ... } export var osmPointTags = {}; diff --git a/modules/presets/index.js b/modules/presets/index.js index 79cfab133..8f2860c5a 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -4,7 +4,7 @@ import { prefs } from '../core/preferences'; import { fileFetcher } from '../core/file_fetcher'; import { locationManager } from '../core/LocationManager'; -import { osmNodeGeometriesForTags, osmSetAreaKeys, osmSetPointTags, osmSetVertexTags } from '../osm/tags'; +import { osmNodeGeometriesForTags, osmSetAreaKeys, osmSetLineTags, osmSetPointTags, osmSetVertexTags } from '../osm/tags'; import { presetCategory } from './category'; import { presetCollection } from './collection'; import { presetField } from './field'; @@ -73,6 +73,7 @@ export function presetIndex() { fields: vals[3] }); osmSetAreaKeys(_this.areaKeys()); + osmSetLineTags(_this.lineTags()); osmSetPointTags(_this.pointTags()); osmSetVertexTags(_this.vertexTags()); }); @@ -341,6 +342,26 @@ export function presetIndex() { }; + _this.lineTags = () => { + return _this.collection.filter((lineTags, d) => { + // ignore name-suggestion-index, deprecated, and generic presets + if (d.suggestion || d.replacement || d.searchable === false) return lineTags; + + // only care about the primary tag + const keys = d.tags && Object.keys(d.tags); + const key = keys && keys.length && keys[0]; // pick the first tag + if (!key) return lineTags; + + // if this can be a line + if (d.geometry.indexOf('line') !== -1) { + lineTags[key] = lineTags[key] || []; + lineTags[key].push(d.tags); + } + return lineTags; + }, {}); + }; + + _this.pointTags = () => { return _this.collection.reduce((pointTags, d) => { // ignore name-suggestion-index, deprecated, and generic presets diff --git a/modules/validations/mismatched_geometry.js b/modules/validations/mismatched_geometry.js index 2eec02ff2..661190e3b 100644 --- a/modules/validations/mismatched_geometry.js +++ b/modules/validations/mismatched_geometry.js @@ -20,14 +20,15 @@ export function validationMismatchedGeometry() { if (entity.type !== 'way' || entity.isClosed()) return null; var tagSuggestingArea = entity.tagSuggestingArea(); + if (!tagSuggestingArea) { return null; } var asLine = presetManager.matchTags(tagSuggestingArea, 'line'); var asArea = presetManager.matchTags(tagSuggestingArea, 'area'); - if (asLine && asArea && (asLine === asArea)) { - // these tags also allow lines and making this an area wouldn't matter + if (asLine && asArea && asLine === asArea) { + // this tag also allows lines and making this an area wouldn't matter return null; } @@ -82,6 +83,21 @@ export function validationMismatchedGeometry() { var tagSuggestingArea = tagSuggestingLineIsArea(entity); if (!tagSuggestingArea) return null; + var validAsLine = false; + var presetAsLine = presetManager.matchTags(entity.tags, 'line'); + if (presetAsLine) { + validAsLine = true; + var key = Object.keys(tagSuggestingArea)[0]; + if (presetAsLine.tags[key] && presetAsLine.tags[key] === '*') { + // only matches a fallback preset of the tag which is suggesting to be an area + validAsLine = false; + } + if (Object.keys(presetAsLine.tags).length === 0) { + // only matches the fallback "line" preset + validAsLine = false; + } + } + return new validationIssue({ type: type, subtype: 'area_as_line', @@ -103,10 +119,13 @@ export function validationMismatchedGeometry() { var entity = context.entity(this.entityIds[0]); var connectEndsOnClick = makeConnectEndpointsFixOnClick(entity, context.graph()); - fixes.push(new validationIssueFix({ - title: t.append('issues.fix.connect_endpoints.title'), - onClick: connectEndsOnClick - })); + if (!validAsLine) { + // only suggest to "connect the ends" if the feature is not also valid as a line + fixes.push(new validationIssueFix({ + title: t.append('issues.fix.connect_endpoints.title'), + onClick: connectEndsOnClick + })); + } fixes.push(new validationIssueFix({ icon: 'iD-operation-delete',