Fix some cases where a degenerate way was created

This commit is contained in:
John Firebaugh
2013-01-23 12:08:25 -05:00
parent 292c916cb1
commit 278bb4a51c
7 changed files with 103 additions and 31 deletions
+6 -1
View File
@@ -5,7 +5,12 @@ iD.actions.DeleteNode = function(nodeId) {
graph.parentWays(node)
.forEach(function(parent) {
graph = graph.replace(parent.removeNode(nodeId));
parent = parent.removeNode(nodeId);
graph = graph.replace(parent);
if (parent.isDegenerate()) {
graph = iD.actions.DeleteWay(parent.id)(graph);
}
});
graph.parentRelations(node)
+4
View File
@@ -58,6 +58,10 @@ _.extend(iD.Way.prototype, {
!this.tags.barrier);
},
isDegenerate: function() {
return _.uniq(this.nodes).length < (this.isArea() ? 3 : 2);
},
geometry: function() {
return this.isArea() ? 'area' : 'line';
},
+24 -13
View File
@@ -12,9 +12,8 @@ iD.modes.DrawArea = function(wayId) {
history = mode.history,
controller = mode.controller,
way = history.graph().entity(wayId),
headId = (way.nodes.length == 1) ?
way.nodes[0] :
way.nodes[way.nodes.length - 2],
index = way.nodes.length - 1,
headId = way.nodes[index - 1],
tailId = way.first(),
node = iD.Node({loc: map.mouseCoordinates()});
@@ -24,12 +23,20 @@ iD.modes.DrawArea = function(wayId) {
history.perform(
iD.actions.AddNode(node),
iD.actions.AddWayNode(way.id, node.id, -1));
iD.actions.AddWayNode(way.id, node.id, index));
surface.selectAll('.way, .node')
.filter(function (d) { return d.id === wayId || d.id === node.id; })
.classed('active', true);
function ReplaceTemporaryNode(replacementId) {
return function(graph) {
graph = graph.replace(graph.entity(wayId).updateNode(replacementId, index));
graph = graph.remove(node);
return graph;
}
}
function mousemove() {
history.replace(iD.actions.MoveNode(node.id, map.mouseCoordinates()));
}
@@ -55,8 +62,7 @@ iD.modes.DrawArea = function(wayId) {
} else if (datum.type === 'node' && datum.id !== node.id) {
// connect the way to an existing node
history.replace(
iD.actions.DeleteNode(node.id),
iD.actions.AddWayNode(way.id, datum.id, -1),
ReplaceTemporaryNode(datum.id),
way.nodes.length > 2 ? 'added to an area' : '');
controller.enter(iD.modes.DrawArea(wayId));
@@ -77,13 +83,11 @@ iD.modes.DrawArea = function(wayId) {
iD.actions.DeleteNode(node.id),
iD.actions.DeleteNode(headId));
if (history.graph().fetch(wayId).nodes.length === 2) {
history.replace(
iD.actions.DeleteNode(way.nodes[0]),
iD.actions.DeleteWay(wayId));
controller.enter(iD.modes.Browse());
} else {
if (history.graph().entity(wayId)) {
controller.enter(iD.modes.DrawArea(wayId));
} else {
// The way was deleted because it had too few nodes.
controller.enter(iD.modes.Browse());
}
}
@@ -95,8 +99,15 @@ iD.modes.DrawArea = function(wayId) {
function ret() {
d3.event.preventDefault();
history.replace(iD.actions.DeleteNode(node.id));
controller.enter(iD.modes.Select(way, true));
if (history.graph().entity(wayId)) {
controller.enter(iD.modes.Select(way, true));
} else {
// The way was deleted because it had too few nodes.
controller.enter(iD.modes.Browse());
}
}
surface
+23 -10
View File
@@ -13,7 +13,7 @@ iD.modes.DrawLine = function(wayId, direction) {
controller = mode.controller,
way = history.graph().entity(wayId),
node = iD.Node({loc: map.mouseCoordinates()}),
index = (direction === 'forward') ? undefined : 0,
index = (direction === 'forward') ? way.nodes.length : 0,
headId = (direction === 'forward') ? way.last() : way.first(),
tailId = (direction === 'forward') ? way.first() : way.last();
@@ -35,6 +35,14 @@ iD.modes.DrawLine = function(wayId, direction) {
.filter(function (d) { return d.id === wayId || d.id === node.id; })
.classed('active', true);
function ReplaceTemporaryNode(replacementId) {
return function(graph) {
graph = graph.replace(graph.entity(wayId).updateNode(replacementId, index));
graph = graph.remove(node);
return graph;
}
}
function mousemove() {
history.replace(iD.actions.MoveNode(node.id, map.mouseCoordinates()));
}
@@ -46,8 +54,7 @@ iD.modes.DrawLine = function(wayId, direction) {
// connect the way in a loop
if (way.nodes.length > 2) {
history.replace(
iD.actions.DeleteNode(node.id),
iD.actions.AddWayNode(wayId, tailId, index),
ReplaceTemporaryNode(tailId),
'added to a line');
controller.enter(iD.modes.Select(way, true));
@@ -66,8 +73,7 @@ iD.modes.DrawLine = function(wayId, direction) {
} else if (datum.type === 'node' && datum.id !== node.id) {
// connect the way to an existing node
history.replace(
iD.actions.DeleteNode(node.id),
iD.actions.AddWayNode(wayId, datum.id, index),
ReplaceTemporaryNode(datum.id),
'added to a line');
controller.enter(iD.modes.DrawLine(wayId, direction));
@@ -106,11 +112,11 @@ iD.modes.DrawLine = function(wayId, direction) {
iD.actions.DeleteNode(node.id),
iD.actions.DeleteNode(headId));
if (history.graph().fetch(wayId).nodes.length === 0) {
history.replace(iD.actions.DeleteWay(wayId));
controller.enter(iD.modes.Browse());
} else {
if (history.graph().entity(wayId)) {
controller.enter(iD.modes.DrawLine(wayId, direction));
} else {
// The way was deleted because it had too few nodes.
controller.enter(iD.modes.Browse());
}
}
@@ -122,8 +128,15 @@ iD.modes.DrawLine = function(wayId, direction) {
function ret() {
d3.event.preventDefault();
history.replace(iD.actions.DeleteNode(node.id));
controller.enter(iD.modes.Select(way, true));
if (history.graph().entity(wayId)) {
controller.enter(iD.modes.Select(way, true));
} else {
// The way was deleted because it had too few nodes.
controller.enter(iD.modes.Browse());
}
}
surface
+3 -7
View File
@@ -15,13 +15,9 @@ iD.modes.Select = function(entity, initial) {
iD.actions.DeleteWay(entity.id),
'deleted a way');
} else if (entity.type === 'node') {
var parents = mode.history.graph().parentWays(entity),
operations = [iD.actions.DeleteNode(entity.id)];
parents.forEach(function(parent) {
if (_.uniq(parent.nodes).length === 1) operations.push(iD.actions.DeleteWay(parent.id));
});
mode.history.perform.apply(mode.history,
operations.concat(['deleted a node']));
mode.history.perform(
iD.actions.DeleteNode(entity.id),
'deleted a node');
}
mode.controller.exit();
+18
View File
@@ -24,4 +24,22 @@ describe("iD.actions.DeleteNode", function () {
graph = action(iD.Graph([node1, node2, relation]));
expect(graph.entity(relation.id).members).to.eql([{ id: node2.id }]);
});
it("deletes parent ways that would otherwise have less than two nodes", function () {
var node1 = iD.Node(),
node2 = iD.Node(),
way = iD.Way({nodes: [node1.id, node2.id]}),
action = iD.actions.DeleteNode(node1.id),
graph = action(iD.Graph([node1, node2, way]));
expect(graph.entity(way.id)).to.be.undefined;
});
it("deletes degenerate circular ways", function () {
var node1 = iD.Node(),
node2 = iD.Node(),
way = iD.Way({nodes: [node1.id, node2.id, node1.id]}),
action = iD.actions.DeleteNode(node2.id),
graph = action(iD.Graph([node1, node2, way]));
expect(graph.entity(way.id)).to.be.undefined;
});
});
+25
View File
@@ -117,6 +117,31 @@ describe('iD.Way', function() {
});
});
describe("#isDegenerate", function() {
it("returns true for a linear way with zero or one nodes", function () {
expect(iD.Way({nodes: []}).isDegenerate()).to.equal(true);
expect(iD.Way({nodes: ['a']}).isDegenerate()).to.equal(true);
});
it("returns true for a circular way with only one unique node", function () {
expect(iD.Way({nodes: ['a', 'a']}).isDegenerate()).to.equal(true);
});
it("returns false for a linear way with two or more nodes", function () {
expect(iD.Way({nodes: ['a', 'b']}).isDegenerate()).to.equal(false);
});
it("returns true for an area with zero, one, or two unique nodes", function () {
expect(iD.Way({tags: {area: 'yes'}, nodes: []}).isDegenerate()).to.equal(true);
expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'a']}).isDegenerate()).to.equal(true);
expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'b', 'a']}).isDegenerate()).to.equal(true);
});
it("returns false for an area with three or more unique nodes", function () {
expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'b', 'c', 'a']}).isDegenerate()).to.equal(false);
});
});
describe("#geometry", function() {
it("returns 'line' when the way is not an area", function () {
expect(iD.Way().geometry()).to.equal('line');