Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JOSM #21944 - Add 'replace selected' action to relation editor #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions resources/images/dialogs/relation/replaceselectedright.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import org.openstreetmap.josm.gui.dialogs.relation.actions.RefreshAction;
import org.openstreetmap.josm.gui.dialogs.relation.actions.RemoveAction;
import org.openstreetmap.josm.gui.dialogs.relation.actions.RemoveSelectedAction;
import org.openstreetmap.josm.gui.dialogs.relation.actions.ReplaceSelectedAction;
import org.openstreetmap.josm.gui.dialogs.relation.actions.ReverseAction;
import org.openstreetmap.josm.gui.dialogs.relation.actions.SelectAction;
import org.openstreetmap.josm.gui.dialogs.relation.actions.SelectPrimitivesForSelectedMembersAction;
Expand Down Expand Up @@ -737,10 +738,13 @@ protected static JToolBar buildSelectionControlButtonToolbar(IRelationEditorActi
new AddSelectedAtEndAction(editorAccess)
));
groups.add(buildNativeGroup(20,
new ReplaceSelectedAction(editorAccess)
));
groups.add(buildNativeGroup(30,
new SelectedMembersForSelectionAction(editorAccess),
new SelectPrimitivesForSelectedMembersAction(editorAccess)
));
groups.add(buildNativeGroup(30,
groups.add(buildNativeGroup(40,
new RemoveSelectedAction(editorAccess)
));
groups.addAll(RelationEditorHooks.getSelectActions());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,42 @@ public void updateRole(int[] idx, String role) {
addToSelectedMembers(selected);
}

/**
* updates the referenced primitive of the members given by the index in <code>index</code>
*
* @param index the index to update
* @param newPrimitive the new primitive
Woazboat marked this conversation as resolved.
Show resolved Hide resolved
* @since xxx
*/
public void updateMemberPrimitive(int index, OsmPrimitive newPrimitive) {
if (index >= members.size()) {
return;
}

RelationMember newMember = new RelationMember(members.get(index).getRole(), newPrimitive);
updateMember(index, newMember);
}

/**
* replace the member at <code>index</code> with a new one
*
* @param index the index to update
* @param newMember the new member
Woazboat marked this conversation as resolved.
Show resolved Hide resolved
* @since xxx
*/
public void updateMember(int index, RelationMember newMember) {
if (index >= members.size()) {
return;
}

RelationMember oldMember = members.get(index);
if (oldMember.equals(newMember))
return;

setValue(index, newMember);
fireTableDataChanged();
}

/**
* Get the currently selected relation members
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,61 @@ protected boolean isPotentialDuplicate(OsmPrimitive primitive) {
return editorAccess.getMemberTableModel().hasMembersReferringTo(Collections.singleton(primitive));
}

/**
* Check and filter a list of primitives before adding them as relation members.
* Prompt users for confirmation when duplicates are detected and prevent relation loops.
*
* @param primitives The primitives to be checked and filtered
* @return The primitives to add to the relation. Never {@code null}, but may be an empty list.
* @throws AddAbortException when a relation loop is detected
*/
protected List<OsmPrimitive> filterConfirmedPrimitives(List<OsmPrimitive> primitives) throws AddAbortException {
return filterConfirmedPrimitives(primitives, false);
}

/**
* Check and filter a list of primitives before adding them as relation members.
* Prompt users for confirmation when duplicates are detected and prevent relation loops.
*
* @param primitives The primitives to be checked and filtered
* @param abortOnSkip If the user decides to not add a primitive or adding a primitive would
* cause a relation loop, abort (throw {@code AddAbortException})
* @return The primitives to add to the relation. Never {@code null}, but may be an empty list.
* @throws AddAbortException when a relation loop is detected or {@code abortOnSkip} is
* {@code true} <i>and</i> the user decides to not add a primitive.
* @since xxx
*/
protected List<OsmPrimitive> filterConfirmedPrimitives(List<OsmPrimitive> primitives, boolean abortOnSkip) throws AddAbortException {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add javadocs here?

Suggested change
protected List<OsmPrimitive> filterConfirmedPrimitives(List<OsmPrimitive> primitives, boolean abortOnSkip) throws AddAbortException {
/**
* Filter confirmed primitives
* @param primitives The primitives to filter on
* @param abortOnSkip If the user decides to not add a primitive, abort (throw {@code AddAbortException})
* @return The primitives to add to the relation. Never {@code null}, but may be an empty list.
* @throws AddAbortException when {@code abortOnSkip} is {@code true} <i>and</i> the user decides to not add a primitive.
* @since xxx
*/
protected List<OsmPrimitive> filterConfirmedPrimitives(List<OsmPrimitive> primitives, boolean abortOnSkip) throws AddAbortException {

You'll want to check this with ant pmd checkstyle, but it should be right.

As a side note, I'd prefer to make this method more generic (e.g. protected <O extends IPrimitive> List<O> filterConfirmedPrimitives(List<O> primitives, boolean abortOnSkip) throws AddAbortException), but I don't know how hard it would be to do (I suspect there would be a cascading chain of methods to change).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the javadocs. Unfortunately making the method more generic would require rewriting pretty much all of the relation editor code from OsmPrimitive to IPrimitive as far as I can tell from a quick glance.

if (Utils.isEmpty(primitives))
return primitives;
List<OsmPrimitive> ret = new ArrayList<>();
ConditionalOptionPaneUtil.startBulkOperation("add_primitive_to_relation");
for (OsmPrimitive primitive : primitives) {
if (primitive instanceof Relation) {
List<Relation> loop = RelationChecker.checkAddMember(editorAccess.getEditor().getRelation(), (Relation) primitive);
if (!loop.isEmpty() && loop.get(0).equals(loop.get(loop.size() - 1))) {
GenericRelationEditor.warnOfCircularReferences(primitive, loop);
continue;
try {
for (OsmPrimitive primitive : primitives) {
if (primitive instanceof Relation) {
List<Relation> loop = RelationChecker.checkAddMember(editorAccess.getEditor().getRelation(), (Relation) primitive);
if (!loop.isEmpty() && loop.get(0).equals(loop.get(loop.size() - 1))) {
GenericRelationEditor.warnOfCircularReferences(primitive, loop);
if (abortOnSkip) {
throw new AddAbortException();
}
continue;
}
}
}
if (isPotentialDuplicate(primitive)) {
if (GenericRelationEditor.confirmAddingPrimitive(primitive)) {
if (isPotentialDuplicate(primitive)) {
if (GenericRelationEditor.confirmAddingPrimitive(primitive)) {
ret.add(primitive);
} else if (abortOnSkip) {
throw new AddAbortException();
}
} else {
ret.add(primitive);
}
} else {
ret.add(primitive);
}
} finally {
ConditionalOptionPaneUtil.endBulkOperation("add_primitive_to_relation");
}
ConditionalOptionPaneUtil.endBulkOperation("add_primitive_to_relation");

return ret;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.gui.dialogs.relation.actions;

import static org.openstreetmap.josm.tools.I18n.tr;

import java.awt.event.ActionEvent;
import java.util.List;

import org.openstreetmap.josm.data.osm.OsmPrimitive;
import org.openstreetmap.josm.gui.dialogs.relation.GenericRelationEditor.AddAbortException;
import org.openstreetmap.josm.tools.ImageProvider;
import org.openstreetmap.josm.tools.Logging;

/**
* Replace selected relation members with the objects selected in the current dataset
* @since xxx
*/
public class ReplaceSelectedAction extends AddFromSelectionAction {

/**
* Constructs a new {@code ReplaceSelectedAction}.
* @param editorAccess An interface to access the relation editor contents.
*/
public ReplaceSelectedAction(IRelationEditorActionAccess editorAccess) {
super(editorAccess, IRelationEditorUpdateOn.MEMBER_TABLE_SELECTION, IRelationEditorUpdateOn.SELECTION_TABLE_CHANGE);
putValue(SHORT_DESCRIPTION, tr("Replace selected members with selected objects"));
new ImageProvider("dialogs/relation", "replaceselectedright").getResource().attachImageIcon(this, true);
updateEnabledState();
}

@Override
protected void updateEnabledState() {
int numSelected = getSelectionTableModel().getRowCount();
setEnabled(numSelected > 0 &&
numSelected == getMemberTableModel().getSelectedIndices().length);
}

@Override
public void actionPerformed(ActionEvent e) {
try {
int[] selectedMemberIndices = getMemberTableModel().getSelectedIndices();
List<OsmPrimitive> selection = getSelectionTableModel().getSelection();
int numSelectedPrimitives = selection.size();
if (numSelectedPrimitives != selectedMemberIndices.length) {
return;
}

List<OsmPrimitive> filteredSelection = filterConfirmedPrimitives(selection, true);

for (int i = 0; i < selectedMemberIndices.length; i++) {
getMemberTableModel().updateMemberPrimitive(selectedMemberIndices[i], filteredSelection.get(i));
}
} catch (AddAbortException ex) {
Logging.trace(ex);
}
}
}