Index: trunk/src/org/openstreetmap/josm/gui/history/HistoryBrowserModel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/history/HistoryBrowserModel.java (revision 4497) +++ trunk/src/org/openstreetmap/josm/gui/history/HistoryBrowserModel.java (revision 4498) @@ -7,7 +7,8 @@ import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Observable; -import javax.swing.table.DefaultTableModel; +import javax.swing.table.AbstractTableModel; import org.openstreetmap.josm.Main; @@ -15,8 +16,6 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; -import org.openstreetmap.josm.data.osm.PrimitiveId; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationMember; -import org.openstreetmap.josm.data.osm.SimplePrimitiveId; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.event.AbstractDatasetChangedEvent; @@ -40,4 +39,5 @@ import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.tools.CheckParameterUtil; +import org.openstreetmap.josm.tools.Diff; /** @@ -80,8 +80,8 @@ private TagTableModel currentTagTableModel; private TagTableModel referenceTagTableModel; - private NodeListTableModel currentNodeListTableModel; - private NodeListTableModel referenceNodeListTableModel; private RelationMemberTableModel currentRelationMemberTableModel; private RelationMemberTableModel referenceRelationMemberTableModel; + private DiffTableModel referenceNodeListTableModel; + private DiffTableModel currentNodeListTableModel; /** @@ -92,6 +92,6 @@ currentTagTableModel = new TagTableModel(PointInTimeType.CURRENT_POINT_IN_TIME); referenceTagTableModel = new TagTableModel(PointInTimeType.REFERENCE_POINT_IN_TIME); - currentNodeListTableModel = new NodeListTableModel(PointInTimeType.CURRENT_POINT_IN_TIME); - referenceNodeListTableModel = new NodeListTableModel(PointInTimeType.REFERENCE_POINT_IN_TIME); + referenceNodeListTableModel = new DiffTableModel(); + currentNodeListTableModel = new DiffTableModel(); currentRelationMemberTableModel = new RelationMemberTableModel(PointInTimeType.CURRENT_POINT_IN_TIME); referenceRelationMemberTableModel = new RelationMemberTableModel(PointInTimeType.REFERENCE_POINT_IN_TIME); @@ -146,5 +146,5 @@ if (primitive == null) return false; if (primitive.isNew() || !primitive.isUsable()) return false; - + //try creating a history primitive. if that fails, the primitive cannot be used. try { @@ -196,4 +196,5 @@ protected void fireModelChange() { + initNodeListTableModels(); setChanged(); notifyObservers(); @@ -216,7 +217,20 @@ } - protected void initNodeListTabeModels() { + /** + * Should be called everytime either reference of current changes to update the diff. + * TODO: Maybe rename to reflect this? eg. updateNodeListTableModels + */ + protected void initNodeListTableModels() { + + if(current.getType() != OsmPrimitiveType.WAY || reference.getType() != OsmPrimitiveType.WAY) + return; + TwoColumnDiff diff = new TwoColumnDiff( + ((HistoryWay)reference).getNodes().toArray(), + ((HistoryWay)current).getNodes().toArray()); + referenceNodeListTableModel.setRows(diff.referenceDiff); + currentNodeListTableModel.setRows(diff.currentDiff); + + referenceNodeListTableModel.fireTableDataChanged(); currentNodeListTableModel.fireTableDataChanged(); - referenceNodeListTableModel.fireTableDataChanged(); } @@ -244,5 +258,5 @@ } - public NodeListTableModel getNodeListTableModel(PointInTimeType pointInTimeType) throws IllegalArgumentException { + public DiffTableModel getNodeListTableModel(PointInTimeType pointInTimeType) throws IllegalArgumentException { CheckParameterUtil.ensureParameterNotNull(pointInTimeType, "pointInTimeType"); if (pointInTimeType.equals(PointInTimeType.CURRENT_POINT_IN_TIME)) @@ -290,5 +304,5 @@ this.reference = reference; initTagTableModels(); - initNodeListTabeModels(); + initNodeListTableModels(); initMemberListTableModels(); setChanged(); @@ -319,5 +333,5 @@ this.current = current; initTagTableModels(); - initNodeListTabeModels(); + initNodeListTableModels(); initMemberListTableModels(); setChanged(); @@ -378,5 +392,5 @@ * */ - public class VersionTableModel extends DefaultTableModel{ + public class VersionTableModel extends AbstractTableModel { private VersionTableModel() { @@ -459,4 +473,9 @@ return p; } + + @Override + public int getColumnCount() { + return 1; + } } @@ -466,5 +485,5 @@ * */ - public class TagTableModel extends DefaultTableModel { + public class TagTableModel extends AbstractTableModel { private ArrayList keys; @@ -554,102 +573,8 @@ return pointInTimeType.equals(PointInTimeType.REFERENCE_POINT_IN_TIME); } - } - - /** - * The table model for the nodes of the version at {@see PointInTimeType#REFERENCE_POINT_IN_TIME} - * or {@see PointInTimeType#CURRENT_POINT_IN_TIME} - * - */ - public class NodeListTableModel extends DefaultTableModel { - - private PointInTimeType pointInTimeType; - - private NodeListTableModel(PointInTimeType pointInTimeType) { - this.pointInTimeType = pointInTimeType; - } - - @Override - public int getRowCount() { - int n = 0; - if (current != null && current.getType().equals(OsmPrimitiveType.WAY)) { - n = ((HistoryWay)current).getNumNodes(); - } - if (reference != null && reference.getType().equals(OsmPrimitiveType.WAY)) { - n = Math.max(n,((HistoryWay)reference).getNumNodes()); - } - return n; - } - - protected HistoryWay getWay() { - if (pointInTimeType.equals(PointInTimeType.CURRENT_POINT_IN_TIME)) { - if (! current.getType().equals(OsmPrimitiveType.WAY)) - return null; - return (HistoryWay)current; - } - if (pointInTimeType.equals(PointInTimeType.REFERENCE_POINT_IN_TIME)) { - if (! reference.getType().equals(OsmPrimitiveType.WAY)) - return null; - return (HistoryWay)reference; - } - - // should not happen - return null; - } - - protected HistoryWay getOppositeWay() { - PointInTimeType opposite = pointInTimeType.opposite(); - if (opposite.equals(PointInTimeType.CURRENT_POINT_IN_TIME)) { - if (! current.getType().equals(OsmPrimitiveType.WAY)) - return null; - return (HistoryWay)current; - } - if (opposite.equals(PointInTimeType.REFERENCE_POINT_IN_TIME)) { - if (! reference.getType().equals(OsmPrimitiveType.WAY)) - return null; - return (HistoryWay)reference; - } - - // should not happen - return null; - } - - @Override - public Object getValueAt(int row, int column) { - HistoryWay way = getWay(); - if (way == null) - return null; - if (row >= way.getNumNodes()) - return null; - return way.getNodes().get(row); - } - - public PrimitiveId getNodeId(int row) { - HistoryWay way = getWay(); - if (way == null) return null; - if (row > way.getNumNodes()) return null; - return new SimplePrimitiveId(way.getNodeId(row), OsmPrimitiveType.NODE); - } - - @Override - public boolean isCellEditable(int row, int column) { - return false; - } - - public boolean isSameInOppositeWay(int row) { - HistoryWay thisWay = getWay(); - HistoryWay oppositeWay = getOppositeWay(); - if (thisWay == null || oppositeWay == null) - return false; - if (row >= oppositeWay.getNumNodes()) - return false; - return thisWay.getNodeId(row) == oppositeWay.getNodeId(row); - } - - public boolean isInOppositeWay(int row) { - HistoryWay thisWay = getWay(); - HistoryWay oppositeWay = getOppositeWay(); - if (thisWay == null || oppositeWay == null) - return false; - return oppositeWay.getNodes().contains(thisWay.getNodeId(row)); + + @Override + public int getColumnCount() { + return 1; } } @@ -661,5 +586,5 @@ */ - public class RelationMemberTableModel extends DefaultTableModel { + public class RelationMemberTableModel extends AbstractTableModel { private PointInTimeType pointInTimeType; @@ -748,4 +673,9 @@ return oppositeRelation.getMembers().contains(thisRelation.getMembers().get(row)); } + + @Override + public int getColumnCount() { + return 1; + } } @@ -918,2 +848,137 @@ } } + +/** + * Simple model storing "diff cells" in a list. Could probably have used a DefaultTableModel instead.. + * + * {@see NodeListDiffTableCellRenderer} + */ +class DiffTableModel extends AbstractTableModel { + private List rows; + + public void setRows(List rows) { + this.rows = rows; + } + + public DiffTableModel(List rows) { + this.rows = rows; + } + public DiffTableModel() { + this.rows = new ArrayList(); + } + @Override + public int getRowCount() { + return rows.size(); + } + + @Override + public int getColumnCount() { + return 1; + } + + @Override + public TwoColumnDiff.Item getValueAt(int rowIndex, int columnIndex) { + return rows.get(rowIndex); + } +} + + +/// Feel free to move me somewhere else. Maybe a bit specific for josm.tools? +/** + * Produces a "two column diff" of two lists. (same as diff -y) + * + * Each list is annotated with the changes relative to the other, and "empty" cells are inserted so the lists are comparable item by item. + * + * diff on [1 2 3 4] [1 a 4 5] yields: + * + * item(SAME, 1) item(SAME, 1) + * item(CHANGED, 2) item(CHANGED, 2) + * item(DELETED, 3) item(EMPTY) + * item(SAME, 4) item(SAME, 4) + * item(EMPTY) item(INSERTED, 5) + * + * @author olejorgenb + */ +class TwoColumnDiff { + public static class Item { + public static final int INSERTED = 1; + public static final int DELETED = 2; + public static final int CHANGED = 3; + public static final int SAME = 4; + public static final int EMPTY = 5; // value should be null + public Item(int state, Object value) { + this.state = state; + this.value = state == EMPTY ? null : value; + } + + public final Object value; + public final int state; + } + + public ArrayList referenceDiff; + public ArrayList currentDiff; + Object[] reference; + Object[] current; + + /** + * The arguments will _not_ be modified + */ + public TwoColumnDiff(Object[] reference, Object[] current) { + this.reference = reference; + this.current = current; + referenceDiff = new ArrayList(); + currentDiff = new ArrayList(); + diff(); + } + private void diff() { + Diff diff = new Diff(reference, current); + Diff.change script = diff.diff_2(false); + twoColumnDiffFromScript(script, reference, current); + } + + /** + * The result from the diff algorithm is a "script" (a compressed description of the changes) + * This method expands this script into a full two column description. + */ + private void twoColumnDiffFromScript(Diff.change script, Object[] a, Object[] b) { + int ia = 0; + int ib = 0; + + while(script != null) { + int deleted = script.deleted; + int inserted = script.inserted; + while(ia < script.line0 && ib < script.line1){ + // System.out.println(" "+a[ia] + "\t "+b[ib]); + Item cell = new Item(Item.SAME, a[ia]); + referenceDiff.add(cell); + currentDiff.add(cell); + ia++; + ib++; + } + + while(inserted > 0 || deleted > 0) { + if(inserted > 0 && deleted > 0) { + // System.out.println("="+a[ia] + "\t="+b[ib]); + referenceDiff.add(new Item(Item.CHANGED, a[ia++])); + currentDiff.add(new Item(Item.CHANGED, b[ib++])); + } else if(inserted > 0) { + // System.out.println("\t+" + b[ib]); + referenceDiff.add(new Item(Item.EMPTY, null)); + currentDiff.add(new Item(Item.INSERTED, b[ib++])); + } else if(deleted > 0) { + // System.out.println("-"+a[ia]); + referenceDiff.add(new Item(Item.DELETED, a[ia++])); + currentDiff.add(new Item(Item.EMPTY, null)); + } + inserted--; + deleted--; + } + script = script.link; + } + while(ia < a.length && ib < b.length) { + // System.out.println((ia < a.length ? " "+a[ia]+"\t" : "\t") + (ib < b.length ? " "+b[ib] : "")); + referenceDiff.add(new Item(Item.SAME, a[ia++])); + currentDiff.add(new Item(Item.SAME, b[ib++])); + } + } +} Index: trunk/src/org/openstreetmap/josm/gui/history/NodeListTableCellRenderer.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/history/NodeListTableCellRenderer.java (revision 4497) +++ trunk/src/org/openstreetmap/josm/gui/history/NodeListTableCellRenderer.java (revision 4498) @@ -14,14 +14,10 @@ import org.openstreetmap.josm.tools.ImageProvider; -/** - * The {@see TableCellRenderer} for a list of nodes in [@see HistoryBrower} - * - * - */ public class NodeListTableCellRenderer extends JLabel implements TableCellRenderer { public final static Color BGCOLOR_EMPTY_ROW = new Color(234,234,234); - public final static Color BGCOLOR_NOT_IN_OPPOSITE = new Color(255,197,197); - public final static Color BGCOLOR_IN_OPPOSITE = new Color(255,234,213); + public final static Color BGCOLOR_DELETED = new Color(255,197,197); + public final static Color BGCOLOR_INSERTED = new Color(0xDD, 0xFF, 0xDD); + public final static Color BGCOLOR_CHANGED = new Color(255,234,213); public final static Color BGCOLOR_SELECTED = new Color(143,170,255); @@ -34,21 +30,28 @@ } - protected void renderNode(HistoryBrowserModel.NodeListTableModel model, Long nodeId, int row, boolean isSelected) { + protected void renderNode(TwoColumnDiff.Item item, boolean isSelected) { String text = ""; Color bgColor = Color.WHITE; - if (nodeId == null) { + setIcon(nodeIcon); + if (item.value != null) { + text = tr("Node {0}", item.value.toString()); + } + switch(item.state) { + case TwoColumnDiff.Item.EMPTY: text = ""; bgColor = BGCOLOR_EMPTY_ROW; setIcon(null); - } else { - text = tr("Node {0}", nodeId.toString()); - setIcon(nodeIcon); - if (model.isSameInOppositeWay(row)) { - bgColor = Color.WHITE; - } else if (model.isInOppositeWay(row)) { - bgColor = BGCOLOR_IN_OPPOSITE; - } else { - bgColor = BGCOLOR_NOT_IN_OPPOSITE; - } + break; + case TwoColumnDiff.Item.CHANGED: + bgColor = BGCOLOR_CHANGED; + break; + case TwoColumnDiff.Item.INSERTED: + bgColor = BGCOLOR_INSERTED; + break; + case TwoColumnDiff.Item.DELETED: + bgColor = BGCOLOR_DELETED; + break; + default: + bgColor = BGCOLOR_EMPTY_ROW; } if (isSelected) { @@ -64,12 +67,6 @@ return this; - HistoryBrowserModel.NodeListTableModel model = getNodeListTableModel(table); - Long nodeId = (Long)value; - renderNode(model, nodeId, row, isSelected); + renderNode((TwoColumnDiff.Item)value, isSelected); return this; } - - protected HistoryBrowserModel.NodeListTableModel getNodeListTableModel(JTable table) { - return (HistoryBrowserModel.NodeListTableModel) table.getModel(); - } } Index: trunk/src/org/openstreetmap/josm/gui/history/NodeListTableColumnModel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/history/NodeListTableColumnModel.java (revision 4497) +++ trunk/src/org/openstreetmap/josm/gui/history/NodeListTableColumnModel.java (revision 4498) @@ -6,4 +6,5 @@ import javax.swing.table.DefaultTableColumnModel; import javax.swing.table.TableColumn; + /** Index: trunk/src/org/openstreetmap/josm/gui/history/NodeListViewer.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/history/NodeListViewer.java (revision 4497) +++ trunk/src/org/openstreetmap/josm/gui/history/NodeListViewer.java (revision 4498) @@ -18,12 +18,14 @@ import javax.swing.JTable; import javax.swing.ListSelectionModel; +import javax.swing.table.TableModel; import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.AutoScaleAction; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.PrimitiveId; +import org.openstreetmap.josm.data.osm.SimplePrimitiveId; import org.openstreetmap.josm.data.osm.history.History; import org.openstreetmap.josm.data.osm.history.HistoryDataSet; -import org.openstreetmap.josm.gui.history.HistoryBrowserModel.NodeListTableModel; import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.tools.ImageProvider; @@ -274,4 +276,11 @@ } + static private PrimitiveId primitiveIdAtRow(TableModel model, int row) { + DiffTableModel castedModel = (DiffTableModel) model; + Long id = (Long)castedModel.getValueAt(row, 0).value; + if(id == null) return null; + return new SimplePrimitiveId(id, OsmPrimitiveType.NODE); + } + class PopupMenuLauncher extends MouseAdapter { private JTable table; @@ -295,6 +304,8 @@ Point p = e.getPoint(); int row = table.rowAtPoint(p); - NodeListTableModel model = (NodeListTableModel) table.getModel(); - PrimitiveId pid = model.getNodeId(row); + + PrimitiveId pid = primitiveIdAtRow(table.getModel(), row); + if (pid == null) + return; popupMenu.prepare(pid); popupMenu.show(e.getComponent(), e.getX(), e.getY()); @@ -309,8 +320,4 @@ this.table = table; showHistoryAction = new ShowHistoryAction(); - } - - protected NodeListTableModel getModel() { - return (NodeListTableModel)table.getModel(); } @@ -319,5 +326,6 @@ if (e.getClickCount() < 2) return; int row = table.rowAtPoint(e.getPoint()); - PrimitiveId pid = getModel().getNodeId(row); + if(row <= 0) return; + PrimitiveId pid = primitiveIdAtRow(table.getModel(), row); if (pid == null) return; Index: trunk/src/org/openstreetmap/josm/gui/history/VersionTable.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/history/VersionTable.java (revision 4497) +++ trunk/src/org/openstreetmap/josm/gui/history/VersionTable.java (revision 4498) @@ -72,5 +72,7 @@ public void valueChanged(ListSelectionEvent e) { DefaultListSelectionModel model = (DefaultListSelectionModel)e.getSource(); - if (model.getMinSelectionIndex() >= 0) { + // For some reason we receive multiple "adjusting" events here even when the source is a simple "set selection" action + // The last and proper event will have getValueIsAdjusting() == false + if (model.getMinSelectionIndex() >= 0 && e.getValueIsAdjusting() == false) { handleSelectCurrentPointInTime(model.getMinSelectionIndex()); }