From 4fa88acc85d7c842eabd42afe05caf4bb6fb3a78 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Tue, 12 Mar 2013 16:21:12 -0700
Subject: [PATCH] Fix connecting adjacent vertices

Can't unconditionally delete the node; it may be a member
of other ways.

I didn't preserve the behavior of dragging a midpoint to
an adjacent node being a no-op. In general we don't try to
eliminate compound operations whose net result is a no-op;
I don't think it's important to do so for this special case.

The degenerate case of connecting the endpoints of a two-vertex
line now results in a point. This is what naturally resulted
from the code, and seems ok.

Fixes #983.
---
 js/id/actions/connect.js     |  6 ++--
 js/id/modes/drag_node.js     | 15 --------
 test/spec/actions/connect.js | 69 ++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/js/id/actions/connect.js b/js/id/actions/connect.js
index aed9882bc..82764e15f 100644
--- a/js/id/actions/connect.js
+++ b/js/id/actions/connect.js
@@ -17,10 +17,12 @@ iD.actions.Connect = function(nodeIds) {
         var survivor = graph.entity(_.last(nodeIds));
 
         for (var i = 0; i < nodeIds.length - 1; i++) {
-            var node = graph.entity(nodeIds[i]), index;
+            var node = graph.entity(nodeIds[i]);
 
             graph.parentWays(node).forEach(function(parent) {
-                graph = graph.replace(parent.replaceNode(node.id, survivor.id));
+                if (!parent.areAdjacent(node.id, survivor.id)) {
+                    graph = graph.replace(parent.replaceNode(node.id, survivor.id));
+                }
             });
 
             graph.parentRelations(node).forEach(function(parent) {
diff --git a/js/id/modes/drag_node.js b/js/id/modes/drag_node.js
index fea02bf86..475f7aef7 100644
--- a/js/id/modes/drag_node.js
+++ b/js/id/modes/drag_node.js
@@ -102,12 +102,6 @@ iD.modes.DragNode = function(context) {
     function end(entity) {
         if (cancelled) return;
 
-        function adjacent(d) {
-            return _.any(context.graph().parentWays(entity).map(function(w) {
-                return w.areAdjacent(d.id, entity.id);
-            }));
-        }
-
         var d = datum();
 
         if (d.type === 'way') {
@@ -117,15 +111,6 @@ iD.modes.DragNode = function(context) {
                 iD.actions.AddVertex(d.id, entity.id, choice.index),
                 connectAnnotation(d));
 
-        } else if (d.type === 'node' && adjacent(d)) {
-            if (wasMidpoint) {
-                context.history().pop();
-            } else {
-                context.replace(
-                    iD.actions.DeleteNode(entity.id),
-                    t('operations.delete.annotation.vertex'));
-            }
-
         } else if (d.type === 'node' && d.id !== entity.id) {
             context.replace(
                 iD.actions.Connect([entity.id, d.id]),
diff --git a/test/spec/actions/connect.js b/test/spec/actions/connect.js
index fc585b43c..683bae623 100644
--- a/test/spec/actions/connect.js
+++ b/test/spec/actions/connect.js
@@ -78,6 +78,75 @@ describe("iD.actions.Connect", function() {
         expect(graph.entity('-').nodes).to.eql(['d', 'b', 'c', 'd']);
     });
 
+    it("merges adjacent nodes", function() {
+        // a --- b --- c
+        //
+        // Connect [b, c]
+        //
+        // Expected result:
+        //
+        // a --- c
+        //
+        var graph = iD.Graph({
+                'a': iD.Node({id: 'a'}),
+                'b': iD.Node({id: 'b'}),
+                'c': iD.Node({id: 'c'}),
+                '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']})
+            });
+
+        graph = iD.actions.Connect(['b', 'c'])(graph);
+
+        expect(graph.entity('-').nodes).to.eql(['a', 'c']);
+        expect(graph.entity('b')).to.be.undefined;
+    });
+
+    it("merges adjacent nodes with connections", function() {
+        // a --- b --- c
+        //       |
+        //       d
+        //
+        // Connect [b, c]
+        //
+        // Expected result:
+        //
+        // a --- c
+        //       |
+        //       d
+        //
+        var graph = iD.Graph({
+                'a': iD.Node({id: 'a'}),
+                'b': iD.Node({id: 'b'}),
+                'c': iD.Node({id: 'c'}),
+                'd': iD.Node({id: 'c'}),
+                '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}),
+                '|': iD.Way({id: '|', nodes: ['b', 'd']})
+
+            });
+
+        graph = iD.actions.Connect(['b', 'c'])(graph);
+
+        expect(graph.entity('-').nodes).to.eql(['a', 'c']);
+        expect(graph.entity('|').nodes).to.eql(['c', 'd']);
+        expect(graph.entity('b')).to.be.undefined;
+    });
+
+    it("deletes a degenerate way", function() {
+        // a --- b
+        //
+        // Connect [a, b]
+        //
+        var graph = iD.Graph({
+                'a': iD.Node({id: 'a'}),
+                'b': iD.Node({id: 'b'}),
+                '-': iD.Way({id: '-', nodes: ['a', 'b']})
+            });
+
+        graph = iD.actions.Connect(['a', 'b'])(graph);
+
+        expect(graph.entity('a')).to.be.undefined;
+        expect(graph.entity('-')).to.be.undefined;
+    });
+
     it("merges tags to the surviving node", function() {
         var graph = iD.Graph({
                 'a': iD.Node({id: 'a', tags: {a: 'a'}}),