Changeset 8219 in josm for trunk/src/org/openstreetmap/josm/gui

Timestamp:

2015-04-18T15:33:28+02:00 (9 years ago)

Author:

simon04

Message:

fix #6994 - History dialog: handle reversed ways better

This is achieved by comparing the reference elements in reversed order and
testing whether this diff requires less deletions+inserts

Location:

trunk/src/org/openstreetmap/josm/gui/history

Files:

• DiffTableModel.java (modified) (3 diffs)
• HistoryBrowserModel.java (modified) (4 diffs)
• NodeListViewer.java (modified) (4 diffs)
• TwoColumnDiff.java (modified) (3 diffs)

trunk/src/org/openstreetmap/josm/gui/history/DiffTableModel.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.gui.history;
+
+import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.util.ArrayList;
@@ -14 +16 @@
  */
 class DiffTableModel extends AbstractTableModel {
-    private List<TwoColumnDiff.Item> rows;
+    private List<TwoColumnDiff.Item> rows = new ArrayList<>();
+    private boolean reversed = false;
 
-    public void setRows(List<TwoColumnDiff.Item> rows) {
+    public void setRows(List<TwoColumnDiff.Item> rows, boolean reversed) {
         this.rows = rows;
-    }
-
-    public DiffTableModel(List<TwoColumnDiff.Item> rows) {
-        this.rows = rows;
-    }
-    public DiffTableModel() {
-        this.rows = new ArrayList<>();
+        this.reversed = reversed;
+        fireTableDataChanged();
     }
     @Override
@@ -34 +32 @@
     public int getColumnCount() {
         return 1;
+    }
+
+    public boolean isReversed() {
+        return reversed;
     }
 

trunk/src/org/openstreetmap/josm/gui/history/HistoryBrowserModel.java

@@ -84 +84 @@
     private HistoryOsmPrimitive latest;
 
-    private VersionTableModel versionTableModel;
-    private TagTableModel currentTagTableModel;
-    private TagTableModel referenceTagTableModel;
-    private DiffTableModel currentRelationMemberTableModel;
-    private DiffTableModel referenceRelationMemberTableModel;
-    private DiffTableModel referenceNodeListTableModel;
-    private DiffTableModel currentNodeListTableModel;
+    private final VersionTableModel versionTableModel;
+    private final TagTableModel currentTagTableModel;
+    private final TagTableModel referenceTagTableModel;
+    private final DiffTableModel currentRelationMemberTableModel;
+    private final DiffTableModel referenceRelationMemberTableModel;
+    private final DiffTableModel referenceNodeListTableModel;
+    private final DiffTableModel currentNodeListTableModel;
 
     /**
@@ -223 +223 @@
      */
     protected void initNodeListTableModels() {
-
         if(current.getType() != OsmPrimitiveType.WAY || reference.getType() != OsmPrimitiveType.WAY)
             return;
@@ -229 +228 @@
                 ((HistoryWay)reference).getNodes().toArray(),
                 ((HistoryWay)current).getNodes().toArray());
-        referenceNodeListTableModel.setRows(diff.referenceDiff);
-        currentNodeListTableModel.setRows(diff.currentDiff);
-
-        referenceNodeListTableModel.fireTableDataChanged();
-        currentNodeListTableModel.fireTableDataChanged();
+        referenceNodeListTableModel.setRows(diff.referenceDiff, diff.referenceReversed);
+        currentNodeListTableModel.setRows(diff.currentDiff, false);
     }
 
@@ -239 +235 @@
         if(current.getType() != OsmPrimitiveType.RELATION || reference.getType() != OsmPrimitiveType.RELATION)
             return;
-
         TwoColumnDiff diff = new TwoColumnDiff(
                 ((HistoryRelation)reference).getMembers().toArray(),
                 ((HistoryRelation)current).getMembers().toArray());
-
-        referenceRelationMemberTableModel.setRows(diff.referenceDiff);
-        currentRelationMemberTableModel.setRows(diff.currentDiff);
-
-        currentRelationMemberTableModel.fireTableDataChanged();
-        referenceRelationMemberTableModel.fireTableDataChanged();
+        referenceRelationMemberTableModel.setRows(diff.referenceDiff, diff.referenceReversed);
+        currentRelationMemberTableModel.setRows(diff.currentDiff, false);
     }
 

trunk/src/org/openstreetmap/josm/gui/history/NodeListViewer.java

@@ -18 +18 @@
 import javax.swing.JTable;
 import javax.swing.ListSelectionModel;
+import javax.swing.event.TableModelEvent;
+import javax.swing.event.TableModelListener;
 import javax.swing.table.TableModel;
 
@@ -60 +62 @@
 
     protected JTable buildReferenceNodeListTable() {
-        JTable table = new JTable(
-                model.getNodeListTableModel(PointInTimeType.REFERENCE_POINT_IN_TIME),
-                new NodeListTableColumnModel()
-        );
+        final DiffTableModel tableModel = model.getNodeListTableModel(PointInTimeType.REFERENCE_POINT_IN_TIME);
+        final NodeListTableColumnModel columnModel = new NodeListTableColumnModel();
+        final JTable table = new JTable(tableModel, columnModel);
+        tableModel.addTableModelListener(newReversedChangeListener(table, columnModel));
         table.setName("table.referencenodelisttable");
         table.setSelectionMode(ListSelectionModel.SINGLE_SELECTION);
@@ -73 +75 @@
 
     protected JTable buildCurrentNodeListTable() {
-        JTable table = new JTable(
-                model.getNodeListTableModel(PointInTimeType.CURRENT_POINT_IN_TIME),
-                new NodeListTableColumnModel()
-        );
+        final DiffTableModel tableModel = model.getNodeListTableModel(PointInTimeType.CURRENT_POINT_IN_TIME);
+        final NodeListTableColumnModel columnModel = new NodeListTableColumnModel();
+        final JTable table = new JTable(tableModel, columnModel);
+        tableModel.addTableModelListener(newReversedChangeListener(table, columnModel));
         table.setName("table.currentnodelisttable");
         table.setSelectionMode(ListSelectionModel.SINGLE_SELECTION);
@@ -83 +85 @@
         table.addMouseListener(new DoubleClickAdapter(table));
         return table;
+    }
+
+    protected TableModelListener newReversedChangeListener(final JTable table, final NodeListTableColumnModel columnModel) {
+        return new TableModelListener() {
+            private Boolean reversed = null;
+            final String nonReversedText = tr("Nodes") + (table.getFont().canDisplay('\u25bc') ? " \u25bc" : " (1-n)");
+            final String reversedText = tr("Nodes") + (table.getFont().canDisplay('\u25b2') ? " \u25b2" : " (n-1)");
+
+            @Override
+            public void tableChanged(TableModelEvent e) {
+                if (e.getSource() instanceof DiffTableModel) {
+                    final DiffTableModel model = (DiffTableModel) e.getSource();
+                    if (reversed == null || reversed != model.isReversed()) {
+                        reversed = model.isReversed();
+                        columnModel.getColumn(0).setHeaderValue(reversed ? reversedText : nonReversedText);
+                        table.getTableHeader().repaint();
+                    }
+                }
+            }
+        };
     }
 

trunk/src/org/openstreetmap/josm/gui/history/TwoColumnDiff.java

@@ -5 +5 @@
 import java.awt.Color;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
@@ -55 +57 @@
     Object[] reference;
     Object[] current;
+    boolean referenceReversed = false;
 
     public TwoColumnDiff(Object[] reference, Object[] current) {
@@ -65 +68 @@
 
     private void diff() {
-        Diff diff = new Diff(reference, current);
-        Diff.Change script = diff.diff_2(false);
-        twoColumnDiffFromScript(script, reference, current);
+        Diff.Change script = new Diff(reference, current).diff_2(false);
+        // attempt diff with reference reversed and test whether less deletions+inserts are required
+        Object[] referenceReversed = Utils.copyArray(reference);
+        Collections.reverse(Arrays.asList(referenceReversed));
+        Diff.Change scriptReversed = new Diff(referenceReversed, current).diff_2(false);
+        if (scriptReversed == null /* reference and current are identical */
+                || script != null && scriptReversed.deleted + scriptReversed.inserted < script.deleted + script.inserted) {
+            this.referenceReversed = true;
+            twoColumnDiffFromScript(scriptReversed, referenceReversed, current);
+        } else {
+            this.referenceReversed = false;
+            twoColumnDiffFromScript(script, reference, current);
+        }
     }