Changeset 14845 in josm for trunk

Timestamp:

2019-03-06T10:17:24+01:00 (6 years ago)

Author:

GerdP

Message:

fix #17401: create a SequenceCommand instead of adding single commands for each fixed error

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/UndoRedoHandler.java (modified) (2 diffs)
• gui/dialogs/ValidatorDialog.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/data/UndoRedoHandler.java

@@ -284 +284 @@
      * Executes the command and add it to the intern command queue.
      * @param c The command to execute. Must not be {@code null}.
-     */
-    public void addNoRedraw(final Command c) {
+     * @param execute true: Execute, else it is assumed that the command was already executed
+     */
+    public void addNoRedraw(final Command c, boolean execute) {
         CheckParameterUtil.ensureParameterNotNull(c, "c");
-        c.executeCommand();
+        if (execute) {
+            c.executeCommand();
+        }
         commands.add(c);
         // Limit the number of commands in the undo list.
@@ -325 +328 @@
 
     /**
+     * Executes the command only if wanted and add it to the intern command queue.
+     * @param c The command to execute. Must not be {@code null}.
+     * @param execute true: Execute, else it is assumed that the command was already executed
+     */
+    public void add(final Command c, boolean execute) {
+        addNoRedraw(c, execute);
+        afterAdd(c);
+
+    }
+
+    /**
      * Executes the command and add it to the intern command queue.
      * @param c The command to execute. Must not be {@code null}.
      */
     public synchronized void add(final Command c) {
-        addNoRedraw(c);
+        addNoRedraw(c, true);
         afterAdd(c);
     }

trunk/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java

@@ -35 +35 @@
 import org.openstreetmap.josm.actions.relation.EditRelationAction;
 import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.UndoRedoHandler;
 import org.openstreetmap.josm.data.osm.DataSelectionListener;
@@ -604 +605 @@
         protected void fixError(TestError error) throws InterruptedException, InvocationTargetException {
             if (error.isFixable()) {
-                final Command fixCommand = error.getFix();
-                if (fixCommand != null) {
-                    SwingUtilities.invokeAndWait(() -> UndoRedoHandler.getInstance().addNoRedraw(fixCommand));
-                    fixCommands.add(fixCommand);
+                if (error.getPrimitives().stream().noneMatch(OsmPrimitive::isDeleted)) {
+                    final Command fixCommand = error.getFix();
+                    if (fixCommand != null) {
+                        SwingUtilities.invokeAndWait(fixCommand::executeCommand);
+                        fixCommands.add(fixCommand);
+                    }
                 }
                 // It is wanted to ignore an error if it said fixable, even if fixCommand was null
@@ -638 +641 @@
                 monitor.subTask(tr("Updating map ..."));
                 SwingUtilities.invokeAndWait(() -> {
-                    UndoRedoHandler.getInstance().afterAdd(fixCommands);
+                    if (!fixCommands.isEmpty()) {
+                        UndoRedoHandler.getInstance().add(
+                                fixCommands.size() > 1 ? new AutofixCommand(fixCommands) : fixCommands.get(0), false);
+                    }
                     invalidateValidatorLayers();
                     tree.resetErrors();
                 });
-            } catch (InterruptedException | InvocationTargetException e) {
+            } catch (InterruptedException e) {
+                tryUndo();
+                throw new JosmRuntimeException(e);
+            } catch (InvocationTargetException e) {
                 // FIXME: signature of realRun should have a generic checked exception we could throw here
                 throw new JosmRuntimeException(e);
             } finally {
+                if (monitor.isCanceled()) {
+                    tryUndo();
+                }
                 monitor.finishTask();
             }
         }
+
+        /**
+         * Undo commands as they were not yet added to the UndoRedo Handler
+         */
+        private void tryUndo() {
+            final DataSet ds = MainApplication.getLayerManager().getActiveDataSet();
+            int i = fixCommands.size() - 1;
+            ds.beginUpdate();
+            for (; i >= 0; i--) {
+                fixCommands.get(i).undoCommand();
+            }
+            ds.endUpdate();
+        }
+
     }
 
@@ -663 +689 @@
         super.destroy();
     }
+
+    private class AutofixCommand extends SequenceCommand {
+        AutofixCommand(Collection<Command> sequenz) {
+            super(tr("auto-fixed validator issues"), sequenz, true);
+            setSequenceComplete(true);
+        }
+
+        @Override
+        public void undoCommand() {
+            getAffectedDataSet().beginUpdate();
+            super.undoCommand();
+            getAffectedDataSet().endUpdate();
+        }
+
+        @Override
+        public boolean executeCommand() {
+            getAffectedDataSet().beginUpdate();
+            boolean rc = super.executeCommand();
+            getAffectedDataSet().endUpdate();
+            return rc;
+        }
+    }
 }