From af7d003b8874b1cec187b72b1607c3e28ba4c82c Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 1 Apr 2013 15:58:10 -0700 Subject: [PATCH] Don't overload constructor parameters --- js/id/actions/disconnect.js | 14 ++++++++--- js/id/actions/split.js | 14 ++++++++--- js/id/operations/disconnect.js | 6 ++++- js/id/operations/split.js | 6 ++++- test/spec/actions/disconnect.js | 8 +++--- test/spec/actions/split.js | 44 ++++++++++++++++----------------- 6 files changed, 58 insertions(+), 34 deletions(-) diff --git a/js/id/actions/disconnect.js b/js/id/actions/disconnect.js index 67336d950..878904dac 100644 --- a/js/id/actions/disconnect.js +++ b/js/id/actions/disconnect.js @@ -12,7 +12,9 @@ // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/UnjoinNodeAction.as // https://github.com/openstreetmap/josm/blob/mirror/src/org/openstreetmap/josm/actions/UnGlueAction.java // -iD.actions.Disconnect = function(nodeId, wayIds, newNodeId) { +iD.actions.Disconnect = function(nodeId, newNodeId) { + var wayIds; + var action = function(graph) { var node = graph.entity(nodeId), replacements = action.replacements(graph); @@ -32,7 +34,7 @@ iD.actions.Disconnect = function(nodeId, wayIds, newNodeId) { parents = graph.parentWays(graph.entity(nodeId)); parents.forEach(function(parent) { - if (wayIds && wayIds.length && wayIds.indexOf(parent.id) === -1) { + if (wayIds && wayIds.indexOf(parent.id) === -1) { keeping = true; return; } @@ -49,9 +51,15 @@ iD.actions.Disconnect = function(nodeId, wayIds, newNodeId) { action.disabled = function(graph) { var replacements = action.replacements(graph); - if (replacements.length === 0 || (wayIds && wayIds.length && wayIds.length !== replacements.length)) + if (replacements.length === 0 || (wayIds && wayIds.length !== replacements.length)) return 'not_connected'; }; + action.limitWays = function(_) { + if (!arguments.length) return wayIds; + wayIds = _; + return action; + }; + return action; }; diff --git a/js/id/actions/split.js b/js/id/actions/split.js index b2df1abae..0feaf9fa9 100644 --- a/js/id/actions/split.js +++ b/js/id/actions/split.js @@ -12,7 +12,9 @@ // Reference: // https://github.com/systemed/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/SplitWayAction.as // -iD.actions.Split = function(nodeId, wayIds, newWayIds) { +iD.actions.Split = function(nodeId, newWayIds) { + var wayIds; + function split(graph, wayA, newWayId) { var wayB = iD.Way({id: newWayId, tags: wayA.tags}), nodesA, @@ -98,7 +100,7 @@ iD.actions.Split = function(nodeId, wayIds, newWayIds) { parents = graph.parentWays(node); return parents.filter(function(parent) { - if (wayIds && wayIds.length && wayIds.indexOf(parent.id) === -1) + if (wayIds && wayIds.indexOf(parent.id) === -1) return false; if (parent.isClosed()) { @@ -117,9 +119,15 @@ iD.actions.Split = function(nodeId, wayIds, newWayIds) { action.disabled = function(graph) { var candidates = action.ways(graph); - if (candidates.length === 0 || (wayIds && wayIds.length && wayIds.length !== candidates.length)) + if (candidates.length === 0 || (wayIds && wayIds.length !== candidates.length)) return 'not_eligible'; }; + action.limitWays = function(_) { + if (!arguments.length) return wayIds; + wayIds = _; + return action; + }; + return action; }; diff --git a/js/id/operations/disconnect.js b/js/id/operations/disconnect.js index 9ec207561..86e4bf78f 100644 --- a/js/id/operations/disconnect.js +++ b/js/id/operations/disconnect.js @@ -4,7 +4,11 @@ iD.operations.Disconnect = function(selection, context) { }); var entityId = vertices[0], - action = iD.actions.Disconnect(entityId, _.without(selection, entityId)); + action = iD.actions.Disconnect(entityId); + + if (selection.length > 1) { + action.limitWays(_.without(selection, entityId)); + } var operation = function() { context.perform(action, t('operations.disconnect.annotation')); diff --git a/js/id/operations/split.js b/js/id/operations/split.js index 6695abccc..b158f172f 100644 --- a/js/id/operations/split.js +++ b/js/id/operations/split.js @@ -4,7 +4,11 @@ iD.operations.Split = function(selection, context) { }); var entityId = vertices[0], - action = iD.actions.Split(entityId, _.without(selection, entityId)); + action = iD.actions.Split(entityId); + + if (selection.length > 1) { + action.limitWays(_.without(selection, entityId)); + } var operation = function() { var annotation; diff --git a/test/spec/actions/disconnect.js b/test/spec/actions/disconnect.js index 925b969c2..fab001edc 100644 --- a/test/spec/actions/disconnect.js +++ b/test/spec/actions/disconnect.js @@ -74,7 +74,7 @@ describe("iD.actions.Disconnect", function () { '|': iD.Way({id: '|', nodes: ['d', 'b']}) }); - graph = iD.actions.Disconnect('b', undefined, 'e')(graph); + graph = iD.actions.Disconnect('b', 'e')(graph); expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c']); expect(graph.entity('|').nodes).to.eql(['d', 'e']); @@ -102,7 +102,7 @@ describe("iD.actions.Disconnect", function () { '|': iD.Way({id: '|', nodes: ['d', 'b']}) }); - graph = iD.actions.Disconnect('b', ['-'], 'e')(graph); + graph = iD.actions.Disconnect('b', 'e').limitWays(['-'])(graph); expect(graph.entity('-').nodes).to.eql(['a', 'e']); expect(graph.entity('=').nodes).to.eql(['b', 'c']); @@ -124,7 +124,7 @@ describe("iD.actions.Disconnect", function () { 'c': iD.Node({id: 'c'}), 'w': iD.Way({id: 'w', nodes: ['a', 'b', 'c', 'a']}) }); - graph = iD.actions.Disconnect('a', undefined, 'd')(graph); + graph = iD.actions.Disconnect('a', 'd')(graph); expect(graph.entity('w').nodes).to.eql(['a', 'b', 'c', 'd']); }); @@ -140,7 +140,7 @@ describe("iD.actions.Disconnect", function () { '|': iD.Way({id: '|', nodes: ['d', 'b']}) }); - graph = iD.actions.Disconnect('b', undefined, 'e')(graph); + graph = iD.actions.Disconnect('b', 'e')(graph); // Immutable loc => should be shared by identity. expect(graph.entity('b').loc).to.equal(loc); diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js index bfdf4ab7c..80dccd305 100644 --- a/test/spec/actions/split.js +++ b/test/spec/actions/split.js @@ -36,7 +36,7 @@ describe("iD.actions.Split", function () { '|': iD.Way({id: '|', nodes: ['c', '*', 'd']}) }); - expect(iD.actions.Split('*', ['-']).disabled(graph)).not.to.be.ok; + expect(iD.actions.Split('*').limitWays(['-']).disabled(graph)).not.to.be.ok; }); it("returns falsy for a self-intersection", function () { @@ -82,7 +82,7 @@ describe("iD.actions.Split", function () { '|': iD.Way({id: '|', nodes: ['c', '*', 'd']}) }); - expect(iD.actions.Split('*', ['-', '=']).disabled(graph)).to.equal('not_eligible'); + expect(iD.actions.Split('*').limitWays(['-', '=']).disabled(graph)).to.equal('not_eligible'); }); }); @@ -102,7 +102,7 @@ describe("iD.actions.Split", function () { '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(graph.entity('-').nodes).to.eql(['a', 'b']); expect(graph.entity('=').nodes).to.eql(['b', 'c']); @@ -117,7 +117,7 @@ describe("iD.actions.Split", function () { '-': iD.Way({id: '-', nodes: ['a', 'b', 'c'], tags: tags}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); // Immutable tags => should be shared by identity. expect(graph.entity('-').tags).to.equal(tags); @@ -146,7 +146,7 @@ describe("iD.actions.Split", function () { '|': iD.Way({id: '|', nodes: ['d', 'b']}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(graph.entity('-').nodes).to.eql(['a', 'b']); expect(graph.entity('=').nodes).to.eql(['b', 'c']); @@ -180,7 +180,7 @@ describe("iD.actions.Split", function () { '|': iD.Way({id: '|', nodes: ['c', '*', 'd']}) }); - graph = iD.actions.Split('*', undefined, ['=', '¦'])(graph); + graph = iD.actions.Split('*', ['=', '¦'])(graph); expect(graph.entity('-').nodes).to.eql(['a', '*']); expect(graph.entity('=').nodes).to.eql(['*', 'b']); @@ -199,17 +199,17 @@ describe("iD.actions.Split", function () { '|': iD.Way({id: '|', nodes: ['c', '*', 'd']}) }); - var g1 = iD.actions.Split('*', ['-'], ['='])(graph); + var g1 = iD.actions.Split('*', ['=']).limitWays(['-'])(graph); expect(g1.entity('-').nodes).to.eql(['a', '*']); expect(g1.entity('=').nodes).to.eql(['*', 'b']); expect(g1.entity('|').nodes).to.eql(['c', '*', 'd']); - var g2 = iD.actions.Split('*', ['|'], ['¦'])(graph); + var g2 = iD.actions.Split('*', ['¦']).limitWays(['|'])(graph); expect(g2.entity('-').nodes).to.eql(['a', '*', 'b']); expect(g2.entity('|').nodes).to.eql(['c', '*']); expect(g2.entity('¦').nodes).to.eql(['*', 'd']); - var g3 = iD.actions.Split('*', ['-', '|'], ['=', '¦'])(graph); + var g3 = iD.actions.Split('*', ['=', '¦']).limitWays(['-', '|'])(graph); expect(g3.entity('-').nodes).to.eql(['a', '*']); expect(g3.entity('=').nodes).to.eql(['*', 'b']); expect(g3.entity('|').nodes).to.eql(['c', '*']); @@ -239,7 +239,7 @@ describe("iD.actions.Split", function () { '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'a', 'd']}) }); - graph = iD.actions.Split('a', undefined, ['='])(graph); + graph = iD.actions.Split('a', ['='])(graph); expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c', 'a']); expect(graph.entity('=').nodes).to.eql(['a', 'd']); @@ -266,19 +266,19 @@ describe("iD.actions.Split", function () { '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) }); - var g1 = iD.actions.Split('a', undefined, ['='])(graph); + var g1 = iD.actions.Split('a', ['='])(graph); expect(g1.entity('-').nodes).to.eql(['a', 'b', 'c']); expect(g1.entity('=').nodes).to.eql(['c', 'd', 'a']); - var g2 = iD.actions.Split('b', undefined, ['='])(graph); + var g2 = iD.actions.Split('b', ['='])(graph); expect(g2.entity('-').nodes).to.eql(['b', 'c', 'd']); expect(g2.entity('=').nodes).to.eql(['d', 'a', 'b']); - var g3 = iD.actions.Split('c', undefined, ['='])(graph); + var g3 = iD.actions.Split('c', ['='])(graph); expect(g3.entity('-').nodes).to.eql(['c', 'd', 'a']); expect(g3.entity('=').nodes).to.eql(['a', 'b', 'c']); - var g4 = iD.actions.Split('d', undefined, ['='])(graph); + var g4 = iD.actions.Split('d', ['='])(graph); expect(g4.entity('-').nodes).to.eql(['d', 'a', 'b']); expect(g4.entity('=').nodes).to.eql(['b', 'c', 'd']); }); @@ -292,7 +292,7 @@ describe("iD.actions.Split", function () { '-': iD.Way({id: '-', tags: {building: 'yes'}, nodes: ['a', 'b', 'c', 'd', 'a']}) }); - graph = iD.actions.Split('a', undefined, ['='])(graph); + graph = iD.actions.Split('a', ['='])(graph); expect(graph.entity('-').tags).to.eql({}); expect(graph.entity('=').tags).to.eql({}); expect(graph.parentRelations(graph.entity('-'))).to.have.length(1); @@ -324,7 +324,7 @@ describe("iD.actions.Split", function () { 'r': iD.Relation({id: 'r', members: [{id: '-', type: 'way', role: 'forward'}]}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(graph.entity('r').members).to.eql([ {id: '-', type: 'way', role: 'forward'}, @@ -353,7 +353,7 @@ describe("iD.actions.Split", function () { 'r': iD.Relation({id: 'r', members: [{id: '-', type: 'way'}, {id: '~', type: 'way'}]}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(_.pluck(graph.entity('r').members, 'id')).to.eql(['-', '=', '~']); }); @@ -379,7 +379,7 @@ describe("iD.actions.Split", function () { 'r': iD.Relation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(_.pluck(graph.entity('r').members, 'id')).to.eql(['~', '=', '-']); }); @@ -393,7 +393,7 @@ describe("iD.actions.Split", function () { 'r': iD.Relation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(_.pluck(graph.entity('r').members, 'id')).to.eql(['~', '-', '=']); }); @@ -423,7 +423,7 @@ describe("iD.actions.Split", function () { {id: 'c', role: 'via'}]}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(graph.entity('r').members).to.eql([ {id: '=', role: 'from'}, @@ -455,7 +455,7 @@ describe("iD.actions.Split", function () { {id: 'c', role: 'via'}]}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(graph.entity('r').members).to.eql([ {id: '~', role: 'from'}, @@ -487,7 +487,7 @@ describe("iD.actions.Split", function () { {id: 'c', role: 'via'}]}) }); - graph = iD.actions.Split('b', undefined, ['='])(graph); + graph = iD.actions.Split('b', ['='])(graph); expect(graph.entity('r').members).to.eql([ {id: '-', role: 'from'},