diff --git a/data/core.yaml b/data/core.yaml index 8297146be..fab61abb3 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -114,6 +114,7 @@ en: not_adjacent: These lines can't be merged because they aren't connected. restriction: These lines can't be merged because at least one is a member of a "{relation}" relation. incomplete_relation: These features can't be merged because at least one hasn't been fully downloaded. + conflicting_tags: These lines can't be merged because some of their tags have conflicting values. move: title: Move description: Move this to a different location. diff --git a/dist/locales/en.json b/dist/locales/en.json index adc0f7421..fbe833758 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -143,7 +143,8 @@ "not_eligible": "These features can't be merged.", "not_adjacent": "These lines can't be merged because they aren't connected.", "restriction": "These lines can't be merged because at least one is a member of a \"{relation}\" relation.", - "incomplete_relation": "These features can't be merged because at least one hasn't been fully downloaded." + "incomplete_relation": "These features can't be merged because at least one hasn't been fully downloaded.", + "conflicting_tags": "These lines can't be merged because some of their tags have conflicting values." }, "move": { "title": "Move", diff --git a/index.html b/index.html index b54c337c6..2e801e4d9 100644 --- a/index.html +++ b/index.html @@ -226,7 +226,7 @@ - + diff --git a/js/id/actions/join.js b/js/id/actions/join.js index 5c7c27210..ef656be05 100644 --- a/js/id/actions/join.js +++ b/js/id/actions/join.js @@ -57,7 +57,9 @@ iD.actions.Join = function(ids) { return 'not_adjacent'; var nodeIds = _.pluck(joined[0].nodes, 'id').slice(1, -1), - relation; + relation, + tags = {}, + conflicting = false; joined[0].forEach(function(way) { var parents = graph.parentRelations(way); @@ -65,10 +67,21 @@ iD.actions.Join = function(ids) { if (parent.isRestriction() && parent.members.some(function(m) { return nodeIds.indexOf(m.id) >= 0; })) relation = parent; }); + + for (var k in way.tags) { + if (!(k in tags)) { + tags[k] = way.tags[k]; + } else if (tags[k] && iD.interestingTag(k) && tags[k] !== way.tags[k]) { + conflicting = true; + } + } }); if (relation) return 'restriction'; + + if (conflicting) + return 'conflicting_tags'; }; return action; diff --git a/js/id/actions/reverse.js b/js/id/actions/reverse.js index 7a1f74be7..461b5d9ac 100644 --- a/js/id/actions/reverse.js +++ b/js/id/actions/reverse.js @@ -29,7 +29,7 @@ http://wiki.openstreetmap.org/wiki/Route#Members http://josm.openstreetmap.de/browser/josm/trunk/src/org/openstreetmap/josm/corrector/ReverseWayTagCorrector.java */ -iD.actions.Reverse = function(wayId) { +iD.actions.Reverse = function(wayId, options) { var replacements = [ [/:right$/, ':left'], [/:left$/, ':right'], [/:forward$/, ':backward'], [/:backward$/, ':forward'] @@ -59,6 +59,8 @@ iD.actions.Reverse = function(wayId) { return value.replace(numeric, function(_, sign) { return sign === '-' ? '' : '-'; }); } else if (key === 'incline' || key === 'direction') { return {up: 'down', down: 'up'}[value] || value; + } else if (options && options.reverseOneway && key === 'oneway') { + return {yes: '-1', '1': '-1', '-1': 'yes'}[value] || value; } else { return {left: 'right', right: 'left'}[value] || value; } diff --git a/js/id/core/entity.js b/js/id/core/entity.js index dcc825fef..b2bb88a7b 100644 --- a/js/id/core/entity.js +++ b/js/id/core/entity.js @@ -116,13 +116,7 @@ iD.Entity.prototype = { }, hasInterestingTags: function() { - return _.keys(this.tags).some(function(key) { - return key !== 'attribution' && - key !== 'created_by' && - key !== 'source' && - key !== 'odbl' && - key.indexOf('tiger:') !== 0; - }); + return _.keys(this.tags).some(iD.interestingTag); }, isHighwayIntersection: function() { diff --git a/js/id/core/oneway_tags.js b/js/id/core/tags.js similarity index 75% rename from js/id/core/oneway_tags.js rename to js/id/core/tags.js index e21d66bdc..50fe79dd4 100644 --- a/js/id/core/oneway_tags.js +++ b/js/id/core/tags.js @@ -29,3 +29,12 @@ iD.oneWayTags = { 'stream': true } }; + +iD.interestingTag = function (key) { + return key !== 'attribution' && + key !== 'created_by' && + key !== 'source' && + key !== 'odbl' && + key.indexOf('tiger:') !== 0; + +}; diff --git a/js/id/geo/multipolygon.js b/js/id/geo/multipolygon.js index a214d52ea..1248c2a96 100644 --- a/js/id/geo/multipolygon.js +++ b/js/id/geo/multipolygon.js @@ -81,7 +81,7 @@ iD.geo.joinWays = function(array, graph) { } function reverse(member) { - return member.tags ? iD.actions.Reverse(member.id)(graph).entity(member.id) : member; + return member.tags ? iD.actions.Reverse(member.id, {reverseOneway: true})(graph).entity(member.id) : member; } while (array.length) { diff --git a/test/index.html b/test/index.html index b1f40fddb..9a38b2222 100644 --- a/test/index.html +++ b/test/index.html @@ -201,7 +201,7 @@ - + diff --git a/test/spec/actions/join.js b/test/spec/actions/join.js index c2d2090ca..4f2bcf496 100644 --- a/test/spec/actions/join.js +++ b/test/spec/actions/join.js @@ -194,6 +194,54 @@ describe("iD.actions.Join", function () { expect(iD.actions.Join(['-', '=']).disabled(graph)).not.to.be.ok; }); + + it("returns 'conflicting_tags' for two entities that have conflicting tags", function () { + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Way({id: '-', nodes: ['a', 'b'], tags: {highway: 'primary'}}), + iD.Way({id: '=', nodes: ['b', 'c'], tags: {highway: 'secondary'}}) + ]); + + expect(iD.actions.Join(['-', '=']).disabled(graph)).to.equal('conflicting_tags'); + }); + + it("takes tag reversals into account when calculating conflicts", function () { + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Way({id: '-', nodes: ['a', 'b'], tags: {'oneway': 'yes'}}), + iD.Way({id: '=', nodes: ['c', 'b'], tags: {'oneway': '-1'}}) + ]); + + expect(iD.actions.Join(['-', '=']).disabled(graph)).not.to.be.ok; + }); + + it("returns falsy for exceptions to tag conflicts: missing tag", function () { + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Way({id: '-', nodes: ['a', 'b'], tags: {highway: 'primary'}}), + iD.Way({id: '=', nodes: ['b', 'c'], tags: {}}) + ]); + + expect(iD.actions.Join(['-', '=']).disabled(graph)).not.to.be.ok; + }); + + it("returns falsy for exceptions to tag conflicts: uninteresting tag", function () { + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Way({id: '-', nodes: ['a', 'b'], tags: {'tiger:cfcc': 'A41'}}), + iD.Way({id: '=', nodes: ['b', 'c'], tags: {'tiger:cfcc': 'A42'}}) + ]); + + expect(iD.actions.Join(['-', '=']).disabled(graph)).not.to.be.ok; + }); }); it("joins a --> b ==> c", function () { diff --git a/test/spec/actions/reverse.js b/test/spec/actions/reverse.js index 9ddc29c7a..bd393e789 100644 --- a/test/spec/actions/reverse.js +++ b/test/spec/actions/reverse.js @@ -23,6 +23,24 @@ describe("iD.actions.Reverse", function () { expect(graph.entity(way.id).tags).to.eql({'oneway': 'yes'}); }); + it("reverses oneway tags if reverseOneway: true is provided", function () { + var graph = iD.Graph([ + iD.Way({id: 'yes', tags: {oneway: 'yes'}}), + iD.Way({id: 'no', tags: {oneway: 'no'}}), + iD.Way({id: '1', tags: {oneway: '1'}}), + iD.Way({id: '-1', tags: {oneway: '-1'}}) + ]); + + expect(iD.actions.Reverse('yes', {reverseOneway: true})(graph) + .entity('yes').tags).to.eql({oneway: '-1'}); + expect(iD.actions.Reverse('no', {reverseOneway: true})(graph) + .entity('no').tags).to.eql({oneway: 'no'}); + expect(iD.actions.Reverse('1', {reverseOneway: true})(graph) + .entity('1').tags).to.eql({oneway: '-1'}); + expect(iD.actions.Reverse('-1', {reverseOneway: true})(graph) + .entity('-1').tags).to.eql({oneway: 'yes'}); + }); + it("transforms *:right=* ⟺ *:left=*", function () { var way = iD.Way({tags: {'cycleway:right': 'lane'}}), graph = iD.Graph([way]); diff --git a/test/spec/geo/multipolygon.js b/test/spec/geo/multipolygon.js index c4a512fc5..49680268d 100644 --- a/test/spec/geo/multipolygon.js +++ b/test/spec/geo/multipolygon.js @@ -86,11 +86,11 @@ describe("iD.geo.joinWays", function() { iD.Node({id: 'b'}), iD.Node({id: 'c'}), iD.Way({id: '-', nodes: ['a', 'b']}), - iD.Way({id: '=', nodes: ['c', 'b'], tags: {'lanes:forward': 2}}) + iD.Way({id: '=', nodes: ['c', 'b'], tags: {'oneway': 'yes', 'lanes:forward': 2}}) ]); var result = iD.geo.joinWays([graph.entity('-'), graph.entity('=')], graph); - expect(result[0][1].tags).to.eql({'lanes:backward': 2}); + expect(result[0][1].tags).to.eql({'oneway': '-1', 'lanes:backward': 2}); }); it("ignores non-way members", function() {