From 5868875833cb5f8a6dc8f705e4a5e4bf9620147e Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Fri, 24 May 2013 11:14:45 -0700
Subject: [PATCH] Split "other" preset into geometry specific fallbacks

This way the name and icon can be geometry-specific.
---
 data/presets.yaml                    | 21 +++++++---
 data/presets/defaults.json           |  8 ++--
 data/presets/presets.json            | 58 +++++++++++++++++-----------
 data/presets/presets/area.json       |  5 +++
 data/presets/presets/line.json       |  5 +++
 data/presets/presets/other.json      |  6 ---
 data/presets/presets/other_area.json |  8 ----
 data/presets/presets/point.json      |  5 +++
 data/presets/presets/relation.json   |  6 +++
 data/presets/presets/vertex.json     |  5 +++
 dist/locales/en.json                 | 28 ++++++++++----
 js/id/presets.js                     | 22 ++++++-----
 js/id/presets/preset.js              |  4 ++
 test/spec/presets.js                 | 18 ++++++---
 test/spec/presets/preset.js          | 22 +++++++----
 15 files changed, 144 insertions(+), 77 deletions(-)
 create mode 100644 data/presets/presets/area.json
 create mode 100644 data/presets/presets/line.json
 delete mode 100644 data/presets/presets/other.json
 delete mode 100644 data/presets/presets/other_area.json
 create mode 100644 data/presets/presets/point.json
 create mode 100644 data/presets/presets/relation.json
 create mode 100644 data/presets/presets/vertex.json

diff --git a/data/presets.yaml b/data/presets.yaml
index 69fafb1dd..8d129cdc2 100644
--- a/data/presets.yaml
+++ b/data/presets.yaml
@@ -370,6 +370,9 @@ en:
       amenity/waste_basket: 
         name: Waste Basket
         terms: "rubbish bin,litter bin,trash can,garbage can"
+      area: 
+        name: Area
+        terms: ""
       barrier: 
         name: Barrier
         terms: ""
@@ -670,6 +673,9 @@ en:
       leisure/swimming_pool: 
         name: Swimming Pool
         terms: ""
+      line: 
+        name: Line
+        terms: ""
       man_made: 
         name: Man Made
         terms: ""
@@ -763,12 +769,6 @@ en:
       office: 
         name: Office
         terms: ""
-      other: 
-        name: Other
-        terms: ""
-      other_area: 
-        name: Other
-        terms: ""
       place: 
         name: Place
         terms: ""
@@ -793,6 +793,9 @@ en:
       place/village: 
         name: Village
         terms: ""
+      point: 
+        name: Point
+        terms: ""
       power: 
         name: Power
         terms: ""
@@ -847,6 +850,9 @@ en:
       railway/tram: 
         name: Tram
         terms: streetcar
+      relation: 
+        name: Relation
+        terms: ""
       shop: 
         name: Shop
         terms: ""
@@ -1096,6 +1102,9 @@ en:
       type/route/tram: 
         name: Tram Route
         terms: ""
+      vertex: 
+        name: Other
+        terms: ""
       waterway: 
         name: Waterway
         terms: ""
diff --git a/data/presets/defaults.json b/data/presets/defaults.json
index 6dcad9e08..afc0062ba 100644
--- a/data/presets/defaults.json
+++ b/data/presets/defaults.json
@@ -8,7 +8,7 @@
         "amenity/place_of_worship",
         "amenity/cafe",
         "amenity/restaurant",
-        "other_area"
+        "area"
     ],
     "line": [
         "category-road",
@@ -16,7 +16,7 @@
         "category-path",
         "category-water",
         "power/line",
-        "other"
+        "line"
     ],
     "point": [
         "leisure/park",
@@ -27,7 +27,7 @@
         "amenity/bar",
         "amenity/bank",
         "shop/supermarket",
-        "other"
+        "point"
     ],
     "vertex": [
         "highway/crossing",
@@ -36,6 +36,6 @@
         "highway/turning_circle",
         "highway/mini_roundabout",
         "highway/motorway_junction",
-        "other"
+        "vertex"
     ]
 }
diff --git a/data/presets/presets.json b/data/presets/presets.json
index 7ac820fbf..09ca90c89 100644
--- a/data/presets/presets.json
+++ b/data/presets/presets.json
@@ -1035,6 +1035,13 @@
         ],
         "name": "Waste Basket"
     },
+    "area": {
+        "name": "Area",
+        "tags": {},
+        "geometry": [
+            "area"
+        ]
+    },
     "barrier": {
         "geometry": [
             "point",
@@ -2511,6 +2518,13 @@
         "icon": "swimming",
         "name": "Swimming Pool"
     },
+    "line": {
+        "name": "Line",
+        "tags": {},
+        "geometry": [
+            "line"
+        ]
+    },
     "man_made": {
         "fields": [
             "man_made"
@@ -2920,28 +2934,6 @@
         "terms": [],
         "name": "Office"
     },
-    "other": {
-        "name": "Other",
-        "tags": {},
-        "geometry": [
-            "point",
-            "vertex",
-            "line",
-            "area",
-            "relation"
-        ],
-        "fields": []
-    },
-    "other_area": {
-        "name": "Other",
-        "tags": {
-            "area": "yes"
-        },
-        "geometry": [
-            "area"
-        ],
-        "fields": []
-    },
     "place": {
         "fields": [
             "place"
@@ -3041,6 +3033,13 @@
         },
         "name": "Village"
     },
+    "point": {
+        "name": "Point",
+        "tags": {},
+        "geometry": [
+            "point"
+        ]
+    },
     "power": {
         "geometry": [
             "point",
@@ -3274,6 +3273,14 @@
         ],
         "name": "Tram"
     },
+    "relation": {
+        "name": "Relation",
+        "icon": "relation",
+        "tags": {},
+        "geometry": [
+            "relation"
+        ]
+    },
     "shop": {
         "icon": "shop",
         "fields": [
@@ -4620,6 +4627,13 @@
         "name": "Tram Route",
         "icon": "route-tram"
     },
+    "vertex": {
+        "name": "Other",
+        "tags": {},
+        "geometry": [
+            "vertex"
+        ]
+    },
     "waterway": {
         "fields": [
             "waterway"
diff --git a/data/presets/presets/area.json b/data/presets/presets/area.json
new file mode 100644
index 000000000..3633bf01b
--- /dev/null
+++ b/data/presets/presets/area.json
@@ -0,0 +1,5 @@
+{
+    "name": "Area",
+    "tags": {},
+    "geometry": ["area"]
+}
\ No newline at end of file
diff --git a/data/presets/presets/line.json b/data/presets/presets/line.json
new file mode 100644
index 000000000..0086b8fbe
--- /dev/null
+++ b/data/presets/presets/line.json
@@ -0,0 +1,5 @@
+{
+    "name": "Line",
+    "tags": {},
+    "geometry": ["line"]
+}
\ No newline at end of file
diff --git a/data/presets/presets/other.json b/data/presets/presets/other.json
deleted file mode 100644
index 25628e2e7..000000000
--- a/data/presets/presets/other.json
+++ /dev/null
@@ -1,6 +0,0 @@
-{
-    "name": "Other",
-    "tags": {},
-    "geometry": ["point", "vertex", "line", "area", "relation"],
-    "fields": []
-}
diff --git a/data/presets/presets/other_area.json b/data/presets/presets/other_area.json
deleted file mode 100644
index 13bacd6cd..000000000
--- a/data/presets/presets/other_area.json
+++ /dev/null
@@ -1,8 +0,0 @@
-{
-    "name": "Other",
-    "tags": {
-        "area": "yes"
-    },
-    "geometry": ["area"],
-    "fields": []
-}
diff --git a/data/presets/presets/point.json b/data/presets/presets/point.json
new file mode 100644
index 000000000..920136cbf
--- /dev/null
+++ b/data/presets/presets/point.json
@@ -0,0 +1,5 @@
+{
+    "name": "Point",
+    "tags": {},
+    "geometry": ["point"]
+}
\ No newline at end of file
diff --git a/data/presets/presets/relation.json b/data/presets/presets/relation.json
new file mode 100644
index 000000000..30cd6aa81
--- /dev/null
+++ b/data/presets/presets/relation.json
@@ -0,0 +1,6 @@
+{
+    "name": "Relation",
+    "icon": "relation",
+    "tags": {},
+    "geometry": ["relation"]
+}
\ No newline at end of file
diff --git a/data/presets/presets/vertex.json b/data/presets/presets/vertex.json
new file mode 100644
index 000000000..b1d9d87d4
--- /dev/null
+++ b/data/presets/presets/vertex.json
@@ -0,0 +1,5 @@
+{
+    "name": "Other",
+    "tags": {},
+    "geometry": ["vertex"]
+}
\ No newline at end of file
diff --git a/dist/locales/en.json b/dist/locales/en.json
index 270b9a78b..5fa89dcae 100644
--- a/dist/locales/en.json
+++ b/dist/locales/en.json
@@ -871,6 +871,10 @@
                 "name": "Waste Basket",
                 "terms": "rubbish bin,litter bin,trash can,garbage can"
             },
+            "area": {
+                "name": "Area",
+                "terms": ""
+            },
             "barrier": {
                 "name": "Barrier",
                 "terms": ""
@@ -1271,6 +1275,10 @@
                 "name": "Swimming Pool",
                 "terms": ""
             },
+            "line": {
+                "name": "Line",
+                "terms": ""
+            },
             "man_made": {
                 "name": "Man Made",
                 "terms": ""
@@ -1395,14 +1403,6 @@
                 "name": "Office",
                 "terms": ""
             },
-            "other": {
-                "name": "Other",
-                "terms": ""
-            },
-            "other_area": {
-                "name": "Other",
-                "terms": ""
-            },
             "place": {
                 "name": "Place",
                 "terms": ""
@@ -1435,6 +1435,10 @@
                 "name": "Village",
                 "terms": ""
             },
+            "point": {
+                "name": "Point",
+                "terms": ""
+            },
             "power": {
                 "name": "Power",
                 "terms": ""
@@ -1507,6 +1511,10 @@
                 "name": "Tram",
                 "terms": "streetcar"
             },
+            "relation": {
+                "name": "Relation",
+                "terms": ""
+            },
             "shop": {
                 "name": "Shop",
                 "terms": ""
@@ -1839,6 +1847,10 @@
                 "name": "Tram Route",
                 "terms": ""
             },
+            "vertex": {
+                "name": "Other",
+                "terms": ""
+            },
             "waterway": {
                 "name": "Waterway",
                 "terms": ""
diff --git a/js/id/presets.js b/js/id/presets.js
index c1d5090f9..2bc80cf19 100644
--- a/js/id/presets.js
+++ b/js/id/presets.js
@@ -5,11 +5,10 @@ iD.presets = function() {
 
     var all = iD.presets.Collection([]),
         defaults = { area: all, line: all, point: all, vertex: all },
+        fallbacks = {},
         fields = {},
         universal = [],
-        recent = iD.presets.Collection([]),
-        other,
-        other_area;
+        recent = iD.presets.Collection([]);
 
     // Index of presets by (geometry, tag key).
     var index = {
@@ -24,7 +23,7 @@ iD.presets = function() {
         var geometry = entity.geometry(resolver),
             geometryMatches = index[geometry],
             best = -1,
-            match = geometry === 'area' ? other_area : other;
+            match;
 
         for (var k in entity.tags) {
             var keyMatches = geometryMatches[k];
@@ -39,7 +38,7 @@ iD.presets = function() {
             }
         }
 
-        return match;
+        return match || fallbacks[geometry];
     };
 
     all.load = function(d) {
@@ -73,8 +72,13 @@ iD.presets = function() {
             };
         }
 
-        other = all.item('other');
-        other_area = all.item('other_area');
+        fallbacks = {
+            point: all.item('point'),
+            vertex: all.item('vertex'),
+            line: all.item('line'),
+            area: all.item('area'),
+            relation: all.item('relation')
+        };
 
         for (var i = 0; i < all.collection.length; i++) {
             var preset = all.collection[i],
@@ -102,11 +106,11 @@ iD.presets = function() {
     all.defaults = function(geometry, n) {
         var rec = recent.matchGeometry(geometry).collection.slice(0, 4),
             def = _.uniq(rec.concat(defaults[geometry].collection)).slice(0, n - 1);
-        return iD.presets.Collection(_.unique(rec.concat(def).concat(geometry === 'area' ? other_area : other)));
+        return iD.presets.Collection(_.unique(rec.concat(def).concat(fallbacks[geometry])));
     };
 
     all.choose = function(preset) {
-        if (preset !== other && preset !== other_area) {
+        if (!preset.isFallback()) {
             recent = iD.presets.Collection(_.unique([preset].concat(recent.collection)));
         }
         return all;
diff --git a/js/id/presets/preset.js b/js/id/presets/preset.js
index 9b0ec527b..691a9dda2 100644
--- a/js/id/presets/preset.js
+++ b/js/id/presets/preset.js
@@ -45,6 +45,10 @@ iD.presets.Preset = function(id, preset, fields) {
         return preset.t('terms', {'default': ''}).split(',');
     };
 
+    preset.isFallback = function() {
+        return Object.keys(preset.tags).length === 0;
+    };
+
     preset.reference = function() {
         var reference = {key: Object.keys(preset.tags)[0]};
 
diff --git a/test/spec/presets.js b/test/spec/presets.js
index f6ffcf706..865031618 100644
--- a/test/spec/presets.js
+++ b/test/spec/presets.js
@@ -1,8 +1,12 @@
 describe("iD.presets", function() {
     var p = {
-        other: {
+        point: {
             tags: {},
-            geometry: ['point', 'vertex', 'line', 'area']
+            geometry: ['point']
+        },
+        line: {
+            tags: {},
+            geometry: ['line']
         },
         residential: {
             tags: {
@@ -29,10 +33,12 @@ describe("iD.presets", function() {
             expect(c.match(way, graph).id).to.eql('residential');
         });
 
-        it("returns an other preset when no tags match", function() {
-            var way = iD.Way({tags: {foo: 'bar'}}),
-                graph = iD.Graph([way]);
-            expect(c.match(way, graph).id).to.eql('other');
+        it("returns the appropriate fallback preset when no tags match", function() {
+            var point = iD.Node(),
+                line = iD.Way({tags: {foo: 'bar'}}),
+                graph = iD.Graph([point, line]);
+            expect(c.match(point, graph).id).to.eql('point');
+            expect(c.match(line, graph).id).to.eql('line');
         });
     });
 });
diff --git a/test/spec/presets/preset.js b/test/spec/presets/preset.js
index 9a3baee39..6e9b46bc4 100644
--- a/test/spec/presets/preset.js
+++ b/test/spec/presets/preset.js
@@ -34,7 +34,7 @@ describe('iD.presets.Preset', function() {
 
 
     it("has optional fields", function() {
-        expect(p.other.fields).to.eql([]);
+        expect(p.point.fields).to.eql([]);
     });
 
     describe('#matchGeometry', function() {
@@ -46,17 +46,17 @@ describe('iD.presets.Preset', function() {
         });
 
         it("returns true if it does match", function() {
-            expect(p.other.matchGeometry('point')).to.equal(true);
+            expect(p.point.matchGeometry('point')).to.equal(true);
         });
     });
 
     describe('#matchScore', function() {
-       it("returns -1 if preset does not match tags", function() {
+        it("returns -1 if preset does not match tags", function() {
             expect(p['highway/residential'].matchScore(w1)).to.equal(-1);
         });
 
-        it("returns 0 for other preset (no match tags)", function() {
-            expect(p.other.matchScore(w1)).to.equal(0);
+        it("returns 0 for fallback presets", function() {
+            expect(p.point.matchScore(w1)).to.equal(0);
         });
 
         it("returns the number of matched tags", function() {
@@ -68,11 +68,19 @@ describe('iD.presets.Preset', function() {
             expect(p.building.matchScore(w4)).to.equal(0.5);
             expect(p.building.matchScore(w5)).to.equal(-1);
         });
+    });
 
+    describe("isFallback", function() {
+        it("returns true if preset has no tags", function() {
+            expect(iD.presets.Preset("area", {name: "Area", tags: {}}).isFallback()).to.equal(true);
+        });
+
+        it("returns false if preset has no tags", function() {
+            expect(p.building.isFallback()).to.equal(false);
+        });
     });
 
     describe('#applyTags', function() {
-
         it("adds match tags", function() {
             expect(p['highway/residential'].applyTags({}, 'area')).to.eql({ highway: 'residential' });
         });
@@ -88,7 +96,6 @@ describe('iD.presets.Preset', function() {
     });
 
     describe('#removeTags', function() {
-
         it('removes match tags', function() {
             expect(p['highway/residential'].removeTags({ highway: 'residential' }, 'area')).to.eql({});
         });
@@ -98,5 +105,4 @@ describe('iD.presets.Preset', function() {
             expect(p['amenity/cafe'].removeTags({ amenity: 'cafe', building: 'yep'}, 'area')).to.eql({ building: 'yep'});
         });
     });
-
 });