Index: trunk/src/org/openstreetmap/josm/actions/DeleteAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/DeleteAction.java (revision 18394) +++ trunk/src/org/openstreetmap/josm/actions/DeleteAction.java (revision 18395) @@ -10,4 +10,5 @@ import java.awt.event.KeyEvent; import java.util.Collection; +import java.util.Collections; import javax.swing.JOptionPane; @@ -24,4 +25,5 @@ import org.openstreetmap.josm.gui.dialogs.DeleteFromRelationConfirmationDialog; import org.openstreetmap.josm.gui.widgets.JMultilineLabel; +import org.openstreetmap.josm.tools.Pair; import org.openstreetmap.josm.tools.Shortcut; @@ -50,6 +52,13 @@ @Override public boolean confirmDeletionFromRelation(Collection references) { + return this.confirmDeletionFromRelation(references, Collections.emptyList()); + } + + @Override + public boolean confirmDeletionFromRelation(Collection references, + Collection> parentsToDelete) { DeleteFromRelationConfirmationDialog dialog = DeleteFromRelationConfirmationDialog.getInstance(); dialog.getModel().populate(references); + dialog.getDeletedRelationsModel().populate(parentsToDelete); dialog.setVisible(true); return !dialog.isCanceled(); Index: trunk/src/org/openstreetmap/josm/command/DeleteCommand.java =================================================================== --- trunk/src/org/openstreetmap/josm/command/DeleteCommand.java (revision 18394) +++ trunk/src/org/openstreetmap/josm/command/DeleteCommand.java (revision 18395) @@ -36,4 +36,5 @@ import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.ImageProvider; +import org.openstreetmap.josm.tools.Pair; import org.openstreetmap.josm.tools.Utils; @@ -102,4 +103,20 @@ */ boolean confirmDeletionFromRelation(Collection references); + + /** + * Confirm before removing a collection of primitives from their parent relations, with the probability of + * deleting the parents as well. + * @param references the list of relation-to-child references + * @param parentsToDelete the list of parents to delete (the boolean part will be {@code true} if the user wants + * to delete the relation). + * @return {@code true} if the user confirms the deletion + * @since 18395 + */ + default boolean confirmDeletionFromRelation(Collection references, + Collection> parentsToDelete) { + // This is a default method. Ensure that all the booleans are false. + parentsToDelete.forEach(pair -> pair.b = false); + return confirmDeletionFromRelation(references); + } } @@ -438,4 +455,21 @@ } + // remove the objects from their parent relations + // + final Set relationsToBeChanged = primitivesToDelete.stream() + .flatMap(p -> p.referrers(Relation.class)) + .collect(Collectors.toSet()); + final Set additionalRelationsToDelete = new HashSet<>(); + for (Relation cur : relationsToBeChanged) { + List newMembers = cur.getMembers(); + cur.getMembersFor(primitivesToDelete).forEach(newMembers::remove); + cmds.add(new ChangeMembersCommand(cur, newMembers)); + // If we don't have any members, we probably ought to delete the relation as well. + if (newMembers.isEmpty()) { + additionalRelationsToDelete.add(cur); + } + } + + // get a confirmation that the objects to delete can be removed from their parent relations // @@ -443,18 +477,15 @@ Set references = RelationToChildReference.getRelationToChildReferences(primitivesToDelete); references.removeIf(ref -> ref.getParent().isDeleted()); - if (!references.isEmpty() && !callback.confirmDeletionFromRelation(references)) { + final Collection> pairedRelations = additionalRelationsToDelete.stream() + /* + * Default to true, so that users have to make a choice to not delete. + * Default implementation converts it to false, so this should be safe. + */ + .map(relation -> new Pair<>(relation, true)).collect(Collectors.toSet()); + if (!references.isEmpty() && !callback.confirmDeletionFromRelation(references, pairedRelations)) { return null; } - } - - // remove the objects from their parent relations - // - final Set relationsToBeChanged = primitivesToDelete.stream() - .flatMap(p -> p.referrers(Relation.class)) - .collect(Collectors.toSet()); - for (Relation cur : relationsToBeChanged) { - List newMembers = cur.getMembers(); - cur.getMembersFor(primitivesToDelete).forEach(newMembers::remove); - cmds.add(new ChangeMembersCommand(cur, newMembers)); + pairedRelations.stream().filter(pair -> Boolean.TRUE.equals(pair.b)).map(pair -> pair.a) + .forEach(primitivesToDelete::add); } Index: trunk/src/org/openstreetmap/josm/gui/dialogs/DeleteFromRelationConfirmationDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/dialogs/DeleteFromRelationConfirmationDialog.java (revision 18394) +++ trunk/src/org/openstreetmap/josm/gui/dialogs/DeleteFromRelationConfirmationDialog.java (revision 18395) @@ -9,4 +9,5 @@ import java.awt.Dimension; import java.awt.FlowLayout; +import java.awt.GridBagLayout; import java.awt.event.ActionEvent; import java.awt.event.WindowAdapter; @@ -35,4 +36,5 @@ import org.openstreetmap.josm.data.osm.NameFormatter; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationToChildReference; import org.openstreetmap.josm.gui.MainApplication; @@ -43,6 +45,9 @@ import org.openstreetmap.josm.gui.util.WindowGeometry; import org.openstreetmap.josm.gui.widgets.HtmlPanel; +import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.I18n; import org.openstreetmap.josm.tools.ImageProvider; +import org.openstreetmap.josm.tools.Pair; +import org.openstreetmap.josm.tools.Utils; /** @@ -69,4 +74,7 @@ /** the data model */ private RelationMemberTableModel model; + /** The data model for deleting relations */ + private RelationDeleteModel deletedRelationsModel; + /** The table to hide/show if the relations to delete are not empty*/ private final HtmlPanel htmlPanel = new HtmlPanel(); private boolean canceled; @@ -75,6 +83,13 @@ protected JPanel buildRelationMemberTablePanel() { JTable table = new JTable(model, new RelationMemberTableColumnModel()); - JPanel pnl = new JPanel(new BorderLayout()); - pnl.add(new JScrollPane(table)); + JPanel pnl = new JPanel(new GridBagLayout()); + pnl.add(new JScrollPane(table), GBC.eol().fill()); + JTable deletedRelationsTable = new JTable(this.deletedRelationsModel, new RelationDeleteTableColumnModel()); + JScrollPane deletedRelationsModelTableScrollPane = new JScrollPane(deletedRelationsTable); + this.deletedRelationsModel.addTableModelListener( + e -> deletedRelationsModelTableScrollPane.setVisible(this.deletedRelationsModel.getRowCount() > 0)); + // Default to not visible + deletedRelationsModelTableScrollPane.setVisible(false); + pnl.add(deletedRelationsModelTableScrollPane, GBC.eol().fill()); return pnl; } @@ -92,4 +107,6 @@ model = new RelationMemberTableModel(); model.addTableModelListener(this); + this.deletedRelationsModel = new RelationDeleteModel(); + this.deletedRelationsModel.addTableModelListener(this); getContentPane().setLayout(new BorderLayout()); getContentPane().add(htmlPanel, BorderLayout.NORTH); @@ -103,6 +120,6 @@ protected void updateMessage() { - int numObjectsToDelete = model.getNumObjectsToDelete(); - int numParentRelations = model.getNumParentRelations(); + int numObjectsToDelete = this.model.getNumObjectsToDelete() + this.deletedRelationsModel.getNumObjectsToDelete(); + int numParentRelations = this.model.getNumParentRelations() + this.deletedRelationsModel.getNumParentRelations(); final String msg1 = trn( "Please confirm to remove {0} object.", @@ -120,5 +137,5 @@ protected void updateTitle() { - int numObjectsToDelete = model.getNumObjectsToDelete(); + int numObjectsToDelete = this.model.getNumObjectsToDelete() + this.deletedRelationsModel.getNumObjectsToDelete(); if (numObjectsToDelete > 0) { setTitle(trn("Deleting {0} object", "Deleting {0} objects", numObjectsToDelete, numObjectsToDelete)); @@ -143,4 +160,13 @@ public RelationMemberTableModel getModel() { return model; + } + + /** + * Replies the data model used for relations that should probably be deleted. + * @return the data model + * @since 18395 + */ + public RelationDeleteModel getDeletedRelationsModel() { + return this.deletedRelationsModel; } @@ -174,4 +200,5 @@ } model.data.clear(); + this.deletedRelationsModel.data.clear(); } super.setVisible(visible); @@ -326,4 +353,133 @@ } + /** + * The table model which manages relations that will be deleted, if their children are deleted. + * @since 18395 + */ + public static class RelationDeleteModel extends DefaultTableModel { + private final transient List> data = new ArrayList<>(); + + @Override + public int getRowCount() { + // This is called in the super constructor. Before we have instantiated the list. Removing the null check + // WILL LEAD TO A SILENT NPE! + if (this.data == null) { + return 0; + } + return this.data.size(); + } + + /** + * Sets the data that should be displayed in the list. + * @param references A list of references to display + */ + public void populate(Collection> references) { + this.data.clear(); + if (references != null) { + this.data.addAll(references); + } + this.data.sort(Comparator.comparing(pair -> pair.a)); + fireTableDataChanged(); + } + + /** + * Gets the list of children that are currently displayed. + * @return The children. + */ + public Set getObjectsToDelete() { + return this.data.stream().filter(relation -> relation.b).map(relation -> relation.a).collect(Collectors.toSet()); + } + + /** + * Gets the number of elements {@link #getObjectsToDelete()} would return. + * @return That number. + */ + public int getNumObjectsToDelete() { + return getObjectsToDelete().size(); + } + + /** + * Gets the set of parent relations + * @return All parent relations of the references + */ + public Set getParentRelations() { + return this.data.stream() + .flatMap(pair -> Utils.filteredCollection(pair.a.getReferrers(), Relation.class).stream()) + .collect(Collectors.toSet()); + } + + /** + * Gets the number of elements {@link #getParentRelations()} would return. + * @return That number. + */ + public int getNumParentRelations() { + return getParentRelations().size(); + } + + @Override + public Object getValueAt(int rowIndex, int columnIndex) { + if (this.data.isEmpty()) { + return null; + } + Pair ref = this.data.get(rowIndex); + switch(columnIndex) { + case 0: return ref.a; + case 1: return ref.b; + default: + assert false : "Illegal column index"; + } + return null; + } + + @Override + public boolean isCellEditable(int row, int column) { + return !this.data.isEmpty() && column == 1; + } + + @Override + public void setValueAt(Object aValue, int row, int column) { + if (this.data.size() > row && column == 1 && aValue instanceof Boolean) { + this.data.get(row).b = ((Boolean) aValue); + } + } + + @Override + public Class getColumnClass(int columnIndex) { + switch (columnIndex) { + case 0: + return Relation.class; + case 1: + return Boolean.class; + default: + return super.getColumnClass(columnIndex); + } + } + } + + private static class RelationDeleteTableColumnModel extends DefaultTableColumnModel { + protected final void createColumns() { + // column 0 - To Delete + TableColumn col = new TableColumn(0); + col.setHeaderValue(tr("Relation")); + col.setResizable(true); + col.setWidth(100); + col.setPreferredWidth(100); + col.setCellRenderer(new PrimitiveRenderer()); + addColumn(col); + + // column 0 - From Relation + col = new TableColumn(1); + final String toDelete = tr("To delete"); + col.setHeaderValue(toDelete); + col.setResizable(true); + col.setPreferredWidth(toDelete.length()); + addColumn(col); + } + + RelationDeleteTableColumnModel() { + createColumns(); + } + } + class OKAction extends AbstractAction { OKAction() {