Merge remote-tracking branch 'zere/master' into HEAD

Conflicts:
	tags.rb

change_bot.rb

@@ -51,6 +51,11 @@ def actions
                  :unclean
                end
 
+      #puts
+      #puts "[#{obj.version}] status:#{status}"
+      #puts "[#{obj.version}] geom:#{obj.geom}"
+      #puts "[#{obj.version}] geom_patch:#{geom_patch}"
+
       # if this is not a clean version, then the only part
       # of the patch we can apply is the deletions, by the
       # 'deletions are always OK' rule.
@@ -89,6 +94,9 @@ def actions
         new_tags.delete(k) if new_tags[k] == v
       end
 
+      #puts "[#{obj.version}] new_geom:#{new_geom}"
+      #puts "[#{obj.version}] obj_geom:#{obj.geom}"
+
       # if the result of applying the patches is any different
       # from the version we actually have, then the object is
       # in a state that we can't display, so redact it.
@@ -104,6 +112,8 @@ def actions
       base_obj.geom = new_geom
       base_obj.tags = new_tags
 
+      #puts "[#{obj.version}] base_obj:#{base_obj}"
+
       prev_obj = obj
     end
 

geom.rb

@@ -106,41 +106,83 @@ def self.create(a, b)
     end
 
     def empty?
-      @diff.all? {|source, elt| source == :c}
+      @diff.empty?
     end
 
     def only_deletes?
-      not @diff.any? {|source, elt| source == :b || source == :d}
+      @diff.all? {|op, idx, elt| op == :delete}
     end
 
     def apply(geom, options = {})
       geom_idx = 0
-      new_geom = Array.new
-
-      @diff.each do |source, elt|
-        case source
-        when :a # exists only in previous - i.e: a delete
-          if geom[geom_idx] == elt
-            geom_idx += 1
+      new_geom = geom.clone
+      # mapping instruction index (i.e: in old geom) to
+      # current index in new_geom.
+      mapping = Hash[(0...(new_geom.length)).map {|i| [i,i]}]
+
+      only_delete = options[:only] == :deleted
+
+      @diff.each do |op, idx, elt|
+        case op
+        when :delete
+          new_idx = mapping[idx]
+          # note: we ignore the role as far as comparing before
+          # applying the patch is concerned - it'll still delete
+          # the same element...
+          unless (new_idx.nil? || 
+                  new_geom[new_idx].nil? ||
+                  new_geom[new_idx].type != elt.type ||
+                  new_geom[new_idx].ref != elt.ref)
+            new_geom.delete_at(new_idx)
+            mapping.delete(idx)
+            mapping.each {|k,v| mapping[k] = v - 1 if v > new_idx}
+            #puts "delete: #{elt}@#{idx}[#{new_idx}] -> #{mapping}"
           end
 
-        when :b # exists only in new version - i.e: an add
-          new_geom << elt unless options[:only] == :deleted
-
-        when :c # exists in both - i.e: unchanged
-          if geom[geom_idx] == elt
-            new_geom << elt
-            geom_idx += 1
+        when :insert
+          unless only_delete
+            new_idx = mapping[idx]
+            new_idx = 0 if new_idx.nil? && mapping.empty?
+            new_idx = mapping.values.max + 1 if new_idx.nil?
+
+            new_geom.insert(new_idx, elt)
+            mapping.each {|k,v| mapping[k] = v + 1 if v >= new_idx}
+            mapping[idx] = new_idx + 1 unless mapping.has_key? idx
+            #puts "insert: #{elt}@#{idx}[#{new_idx}] -> #{mapping}"
           end
 
-        when :d
-          cur = geom[geom_idx]
-          unless cur.nil?
-            from_role, to_role = elt.role
-            if cur.type == elt.type && cur.ref == elt.ref
-              new_role = (options[:only] == :deleted) ? cur.role : to_role
-              new_geom << OSM::Relation::Member[elt.type, elt.ref, new_role]
-              geom_idx += 1
+        when :move
+          unless only_delete
+            old_idx_from, old_idx_to = idx
+
+            new_idx_to = mapping[old_idx_to]
+            new_idx_from = mapping[old_idx_from]
+            unless new_idx_from.nil? || new_geom[new_idx_from] != elt
+              new_idx_to = 0 if new_idx_to.nil? && mapping.empty?
+              new_idx_to = mapping.values.max + 1 if new_idx_to.nil?
+
+              new_geom.insert(new_idx_to, elt)
+              mapping.each {|k,v| mapping[k] = v + 1 if v >= new_idx_to}
+              mapping[old_idx_to] = new_idx_to + 1 unless mapping.has_key? old_idx_to
+
+              # reset new_index_from after mapping change
+              new_idx_from = mapping[old_idx_from]
+              new_geom.delete_at(new_idx_from)
+              mapping.delete(old_idx_from)
+              mapping.each {|k,v| mapping[k] = v - 1 if v > new_idx_from}
+            end
+          end
+
+        when :alter
+          unless only_delete
+            new_idx = mapping[idx]
+            # don't bother comparing the old value of the role,
+            # since that's what we're overwriting anyway.
+            unless (new_idx.nil? || 
+                    new_geom[new_idx].nil? ||
+                    new_geom[new_idx].type != elt[1].type ||
+                    new_geom[new_idx].ref != elt[1].ref)
+              new_geom[new_idx] = elt[1]
             end
           end
         end
@@ -159,36 +201,70 @@ def to_s
 
     private
     def initialize(d)
+      a_idx = 0
       @diff = Array.new
-      current = Array.new
-
-      d.each do |source, elt|
-        # unchanged elements go straight into the diff
-        if source == :c
-          # along with any unmatched elements
-          @diff.concat(current)
-          current = Array.new
-          @diff << [source, elt]
-
-        else
-          # if any of the current elements match this one
-          # except for the role, then consider it as a 
-          # role move instead.
-          idx = current.index {|s,e| s != source && e.type == elt.type && e.ref == elt.ref}
+
+      d.each do |src, elt|
+        case src
+        when :a # element only in A: a delete
+          @diff << [:delete, a_idx, elt]
+          a_idx += 1
+
+        when :b # element only in B: an insert
+          @diff << [:insert, a_idx, elt]
 
-          if idx.nil?
-            # otherwise, queue up the element
-            current << [source, elt]
-
-          else
-            s, e = current.delete_at(idx)
-            roles = (source == :a) ? [elt.role, e.role] : [e.role, elt.role]
-            @diff << [:d, OSM::Relation::Member[elt.type, elt.ref, roles]]
-          end
+        when :c # element in both - ignore
+          a_idx += 1
         end
       end
 
-      @diff.concat(current)
+      # try and find insert-delete pairs where the
+      # element is exactly the same. these are the
+      # moves.
+      moves = @diff.
+        group_by {|op,idx,elt| elt}.
+        select {|elt,vec| (vec.length > 1 && 
+                           vec.any? {|op,idx,el| op == :insert} && 
+                           vec.any? {|op,idx,el| op == :delete})}
+
+      moves.each do |elt, vec|
+        # could be more than 2 - for simplicity 
+        # just assume the first of each delete 
+        # and insert.
+        from = vec.find {|op,idx,elt| op == :delete}
+        to   = vec.find {|op,idx,elt| op == :insert}
+
+        from_idx = @diff.find_index(from)
+        to_idx   = @diff.find_index(to)
+
+        #puts "from_idx:#{from_idx} to_idx:#{to_idx}"
+        @diff[from_idx] = [:move, [from[1], to[1]], elt]
+        @diff.delete_at(to_idx)
+      end
+
+      # now try and detect alterations to the role
+      # which we treat separately from other changes.
+      alters = @diff.
+        select {|op,idx,elt| op != :move}.
+        group_by {|op,idx,elt| [(op == :insert) ? idx - 1 : idx, elt.type, elt.ref]}.
+        select {|x,vec| (vec.length > 1 && 
+                           vec.any? {|op,idx,el| op == :insert} && 
+                           vec.any? {|op,idx,el| op == :delete})}
+
+      alters.each do |x, vec|
+        # could be more than 2 - for simplicity 
+        # just assume the first of each delete 
+        # and insert.
+        from = vec.find {|op,idx,elt| op == :delete}
+        to   = vec.find {|op,idx,elt| op == :insert}
+
+        from_idx = @diff.find_index(from)
+        to_idx   = @diff.find_index(to)
+
+        #puts "from_idx:#{from_idx} to_idx:#{to_idx}"
+        @diff[from_idx] = [:alter, from[1], [from[2], to[2]]]
+        @diff.delete_at(to_idx)        
+      end
     end
   end
 end

osm.rb

@@ -159,6 +159,16 @@ def <=>(o)
         @role <=> o.role
       end
 
+      def hash
+        [@type, @ref, @role].hash
+      end
+
+      def eql?(o)
+        @type.eql?(o.type) &&
+          @ref.eql?(o.ref) &&
+          @role.eql?(o.role)
+      end
+
       def to_s
         "Member[#{@type.inspect},#{@ref},#{@role.inspect}]"
       end

tags.rb

@@ -23,10 +23,13 @@ def self.odbl_clean?(tags)
       # special case for this one misspelling, as it's fairly
       # common to find "obdl" and there's no chance that we're
       # confusing "obdl" with anything else.
-      if k.downcase == "odbl" or k.downcase == "obdl" or k.downcase == "oodbl"
+      if (k.downcase == "odbl" or 
+          k.downcase == "obdl" or
+          k.downcase == "oodbl")
         val = tags[k].downcase
         # tag synonyms for "clean" in this context
         (val == "clean" ||
+         val == "clear"  ||
          val == "true"  ||
          val == "yes"   ||
          val == "clear" ||
@@ -287,9 +290,13 @@ def self.significant_tag?(old_v, new_v)
     # now check for homophones (TODO: is this really appropriate?)
     return false if Text::Metaphone.metaphone(old) == Text::Metaphone.metaphone(new)
 
-    # finally, look for changes in abbreviation.
+    # look for changes in abbreviation.
     return false if Abbrev.equal_expansions(old, new)
 
+    # check if the strings are the same except for whitespace
+    # presence. this would be considered insignificant.
+    return false if old.gsub(/ /,"") == new.gsub(/ /,"")
+
     # otherwise, just look at the strings...
     old != new
   end

test.rb

@@ -14,6 +14,7 @@
 require './test_tags'
 require './test_util'
 require './test_way'
+require './test_geom'
 require './test_needs_clarity'
 require './test_auto_fail'