Changeset 10663 in josm for trunk/src/org/openstreetmap/josm/command

Timestamp:

2016-07-28T01:24:39+02:00 (8 years ago)

Author:

Don-vip

Message:

fix #13223 - Minor command class fixes (patch by michael2402, modified) - gsoc-core

Location:

trunk/src/org/openstreetmap/josm/command

Files:

• ChangeNodesCommand.java (modified) (2 diffs)
• ChangePropertyCommand.java (modified) (4 diffs)
• ChangePropertyKeyCommand.java (modified) (3 diffs)
• ChangeRelationMemberRoleCommand.java (modified) (2 diffs)
• Command.java (modified) (1 diff)
• DeleteCommand.java (modified) (6 diffs)
• MoveCommand.java (modified) (1 diff)
• RotateCommand.java (modified) (2 diffs)
• ScaleCommand.java (modified) (2 diffs)
• SelectCommand.java (modified) (2 diffs)
• TransformNodesCommand.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/command/ChangeNodesCommand.java

@@ -38 +38 @@
         this.way = way;
         this.newNodes = newNodes;
+        if (newNodes.isEmpty()) {
+            throw new IllegalArgumentException("Cannot set nodes to be an empty list.");
+        }
     }
 
@@ -55 +58 @@
     @Override
     public String getDescriptionText() {
-        return tr("Changed nodes of {0}", way.getDisplayName(DefaultNameFormatter.getInstance()));
+        return tr("Change nodes of {0}", way.getDisplayName(DefaultNameFormatter.getInstance()));
     }
 

trunk/src/org/openstreetmap/josm/command/ChangePropertyCommand.java

@@ -156 +156 @@
             String msg;
             Map.Entry<String, String> entry = tags.entrySet().iterator().next();
-            if (entry.getValue() == null) {
+            if (entry.getValue() == null || entry.getValue().isEmpty()) {
                 switch(OsmPrimitiveType.from(primitive)) {
                 case NODE: msg = marktr("Remove \"{0}\" for node ''{1}''"); break;
@@ -175 +175 @@
         } else if (objects.size() > 1 && tags.size() == 1) {
             Map.Entry<String, String> entry = tags.entrySet().iterator().next();
-            if (entry.getValue() == null) {
+            if (entry.getValue() == null || entry.getValue().isEmpty()) {
                 /* I18n: plural form for objects, but value < 2 not possible! */
                 text = trn("Remove \"{0}\" for {1} object", "Remove \"{0}\" for {1} objects", objects.size(), entry.getKey(), objects.size());
@@ -186 +186 @@
             boolean allnull = true;
             for (Map.Entry<String, String> tag : this.tags.entrySet()) {
-                if (tag.getValue() != null) {
+                if (tag.getValue() != null && !tag.getValue().isEmpty()) {
                     allnull = false;
                     break;
@@ -226 +226 @@
                     return Collections.singleton(osm);
                 }
-
             });
         }

trunk/src/org/openstreetmap/josm/command/ChangePropertyKeyCommand.java

@@ -66 +66 @@
             return false; // save old
         for (OsmPrimitive osm : objects) {
-            if (osm.hasKeys()) {
+            if (osm.hasKey(key) || osm.hasKey(newKey)) {
                 osm.setModified(true);
                 String oldValue = osm.get(key);
@@ -87 +87 @@
             NameVisitor v = new NameVisitor();
             objects.get(0).accept(v);
-            text += ' '+tr(v.className)+' '+v.name;
+            text += " "+tr(v.className)+" "+v.name;
         } else {
-            text += ' '+objects.size()+' '+trn("object", "objects", objects.size());
+            text += " "+objects.size()+" "+trn("object", "objects", objects.size());
         }
         return text;
@@ -108 +108 @@
         for (final OsmPrimitive osm : objects) {
             osm.accept(v);
+            final String name = v.name;
+            final Icon icon = v.icon;
             children.add(new PseudoCommand() {
                 @Override
                 public String getDescriptionText() {
-                    return v.name;
+                    return name;
                 }
 
                 @Override
                 public Icon getDescriptionIcon() {
-                    return v.icon;
+                    return icon;
                 }
 

trunk/src/org/openstreetmap/josm/command/ChangeRelationMemberRoleCommand.java

@@ -49 +49 @@
     public boolean executeCommand() {
         if (position < 0 || position >= relation.getMembersCount())
-            return false;
+            return true;
 
         oldRole = relation.getMember(position).getRole();
@@ -62 +62 @@
     @Override
     public void undoCommand() {
-        relation.setMember(position, new RelationMember(oldRole, relation.getMember(position).getMember()));
-        if (oldModified != null) {
-            relation.setModified(oldModified);
+        if (position >= 0 && position < relation.getMembersCount()) {
+            relation.setMember(position, new RelationMember(oldRole, relation.getMember(position).getMember()));
+            if (oldModified != null) {
+                relation.setModified(oldModified);
+            }
         }
     }

trunk/src/org/openstreetmap/josm/command/Command.java

@@ -188 +188 @@
      * the removed layer)
      *
-     * @param oldLayer the old layer
-     * @return true if this command
+     * @param oldLayer the old layer that was removed
+     * @return true if this command is invalid after that layer is removed.
      */
     public boolean invalidBecauselayerRemoved(Layer oldLayer) {
-        if (!(oldLayer instanceof OsmDataLayer))
-            return false;
         return layer == oldLayer;
     }

trunk/src/org/openstreetmap/josm/command/DeleteCommand.java

@@ -51 +51 @@
  */
 public class DeleteCommand extends Command {
+    private static final class DeleteChildCommand implements PseudoCommand {
+        private final OsmPrimitive osm;
+
+        private DeleteChildCommand(OsmPrimitive osm) {
+            this.osm = osm;
+        }
+
+        @Override
+        public String getDescriptionText() {
+            return tr("Deleted ''{0}''", osm.getDisplayName(DefaultNameFormatter.getInstance()));
+        }
+
+        @Override
+        public Icon getDescriptionIcon() {
+            return ImageProvider.get(osm.getDisplayType());
+        }
+
+        @Override
+        public Collection<? extends OsmPrimitive> getParticipatingPrimitives() {
+            return Collections.singleton(osm);
+        }
+
+        @Override
+        public String toString() {
+            return "DeleteChildCommand [osm=" + osm + "]";
+        }
+    }
+
     /**
      * The primitives that get deleted.
@@ -65 +93 @@
     public DeleteCommand(Collection<? extends OsmPrimitive> data) {
         CheckParameterUtil.ensureParameterNotNull(data, "data");
-        if (data.isEmpty())
-            throw new IllegalArgumentException(tr("At least one object to delete required, got empty collection"));
         this.toDelete = data;
         checkConsistency();
@@ -106 +132 @@
         super(layer);
         CheckParameterUtil.ensureParameterNotNull(data, "data");
-        if (data.isEmpty())
-            throw new IllegalArgumentException(tr("At least one object to delete required, got empty collection"));
         this.toDelete = data;
         checkConsistency();
@@ -113 +137 @@
 
     private void checkConsistency() {
+        if (toDelete.isEmpty()) {
+            throw new IllegalArgumentException(tr("At least one object to delete required, got empty collection"));
+        }
         for (OsmPrimitive p : toDelete) {
             if (p == null) {
@@ -216 +243 @@
             List<PseudoCommand> children = new ArrayList<>(toDelete.size());
             for (final OsmPrimitive osm : toDelete) {
-                children.add(new PseudoCommand() {
-
-                    @Override public String getDescriptionText() {
-                        return tr("Deleted ''{0}''", osm.getDisplayName(DefaultNameFormatter.getInstance()));
-                    }
-
-                    @Override public Icon getDescriptionIcon() {
-                        return ImageProvider.get(osm.getDisplayType());
-                    }
-
-                    @Override public Collection<? extends OsmPrimitive> getParticipatingPrimitives() {
-                        return Collections.singleton(osm);
-                    }
-
-                });
+                children.add(new DeleteChildCommand(osm));
             }
             return children;
@@ -441 +454 @@
     }
 
+    /**
+     * Create a command that deletes a single way segment. The way may be split by this.
+     * @param layer The layer the segment is in.
+     * @param ws The way segment that should be deleted
+     * @return A matching command to safely delete that segment.
+     */
     public static Command deleteWaySegment(OsmDataLayer layer, WaySegment ws) {
         if (ws.way.getNodesCount() < 3)

trunk/src/org/openstreetmap/josm/command/MoveCommand.java

@@ -243 +243 @@
     public Collection<Node> getParticipatingPrimitives() {
         return nodes;
+    }
+
+    /**
+     * Gets the offset.
+     * @return The current offset.
+     */
+    protected EastNorth getOffset() {
+        return new EastNorth(x, y);
     }
 

trunk/src/org/openstreetmap/josm/command/RotateCommand.java

@@ -39 +39 @@
      * @param currentEN cuurent eats/north
      */
-    public RotateCommand(Collection<OsmPrimitive> objects, EastNorth currentEN) {
+    public RotateCommand(Collection<? extends OsmPrimitive> objects, EastNorth currentEN) {
         super(objects);
 
@@ -71 +71 @@
 
     /**
+     * Set the rotation angle.
+     * @param rotationAngle The rotate angle
+     */
+    protected void setRotationAngle(double rotationAngle) {
+        this.rotationAngle = rotationAngle;
+    }
+
+    /**
      * Rotate nodes.
      */
     @Override
     protected void transformNodes() {
+        double cosPhi = Math.cos(rotationAngle);
+        double sinPhi = Math.sin(rotationAngle);
         for (Node n : nodes) {
-            double cosPhi = Math.cos(rotationAngle);
-            double sinPhi = Math.sin(rotationAngle);
             EastNorth oldEastNorth = oldStates.get(n).getEastNorth();
             double x = oldEastNorth.east() - pivot.east();

trunk/src/org/openstreetmap/josm/command/ScaleCommand.java

@@ -35 +35 @@
      * @param currentEN cuurent eats/north
      */
-    public ScaleCommand(Collection<OsmPrimitive> objects, EastNorth currentEN) {
+    public ScaleCommand(Collection<? extends OsmPrimitive> objects, EastNorth currentEN) {
         super(objects);
 
@@ -58 +58 @@
         double startDistance = pivot.distance(startEN);
         double currentDistance = pivot.distance(currentEN);
-        scalingFactor = Math.cos(startAngle-endAngle) * currentDistance / startDistance;
+        setScalingFactor(Math.cos(startAngle-endAngle) * currentDistance / startDistance);
         transformNodes();
+    }
+
+    /**
+     * Set the scaling factor
+     * @param scalingFactor The scaling factor.
+     */
+    protected void setScalingFactor(double scalingFactor) {
+        this.scalingFactor = scalingFactor;
     }
 

trunk/src/org/openstreetmap/josm/command/SelectCommand.java

@@ -5 +5 @@
 
 import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.Objects;
 
@@ -28 +30 @@
      */
     public SelectCommand(Collection<OsmPrimitive> newSelection) {
-        this.newSelection = newSelection;
+        if (newSelection == null || newSelection.isEmpty()) {
+            this.newSelection = Collections.emptySet();
+        } else {
+            this.newSelection = new HashSet<>(newSelection);
+        }
     }
 

trunk/src/org/openstreetmap/josm/command/TransformNodesCommand.java

@@ -49 +49 @@
      * @param objects objects to fetch nodes from
      */
-    public TransformNodesCommand(Collection<OsmPrimitive> objects) {
+    public TransformNodesCommand(Collection<? extends OsmPrimitive> objects) {
         this.nodes = AllNodesVisitor.getAllNodes(objects);
         storeOldState();