Changeset 11885 in josm for trunk/src/org/openstreetmap

Timestamp:

2017-04-10T23:49:54+02:00 (8 years ago)

Author:

Don-vip

Message:

add basic unit tests for MergeLayerAction + improve javadoc, code cleanup

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/AbstractMergeAction.java (modified) (6 diffs)
• actions/MergeLayerAction.java (modified) (5 diffs)
• gui/MapFrame.java (modified) (1 diff)
• gui/dialogs/LayerListDialog.java (modified) (4 diffs)
• gui/dialogs/ToggleDialog.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/actions/AbstractMergeAction.java

@@ -10 +10 @@
 
 import javax.swing.DefaultListCellRenderer;
-import javax.swing.Icon;
 import javax.swing.JLabel;
 import javax.swing.JList;
@@ -24 +23 @@
 import org.openstreetmap.josm.tools.Utils;
 
+/**
+ * Abstract superclass of different "Merge" actions.
+ * @since 1890
+ */
 public abstract class AbstractMergeAction extends JosmAction {
 
     /**
      * the list cell renderer used to render layer list entries
-     *
      */
     public static class LayerListCellRenderer extends DefaultListCellRenderer {
@@ -36 +38 @@
             Layer layer = (Layer) value;
             JLabel label = (JLabel) super.getListCellRendererComponent(list, layer.getName(), index, isSelected, cellHasFocus);
-            Icon icon = layer.getIcon();
-            label.setIcon(icon);
+            label.setIcon(layer.getIcon());
             label.setToolTipText(layer.getToolTipText());
             return label;
@@ -45 +46 @@
     /**
      * Constructs a new {@code AbstractMergeAction}.
+     * @param name the action's text as displayed on the menu (if it is added to a menu)
+     * @param iconName the filename of the icon to use
+     * @param tooltip  a longer description of the action that will be displayed in the tooltip. Please note
+     *           that html is not supported for menu actions on some platforms.
+     * @param shortcut a ready-created shortcut object or null if you don't want a shortcut. But you always
+     *            do want a shortcut, remember you can always register it with group=none, so you
+     *            won't be assigned a shortcut unless the user configures one. If you pass null here,
+     *            the user CANNOT configure a shortcut for your action.
+     * @param register register this action for the toolbar preferences?
      */
-    public AbstractMergeAction() {
-        super();
-    }
-
     public AbstractMergeAction(String name, String iconName, String tooltip, Shortcut shortcut, boolean register) {
         super(name, iconName, tooltip, shortcut, register);
     }
 
+    /**
+     * Constructs a new {@code AbstractMergeAction}.
+     * @param name the action's text as displayed on the menu (if it is added to a menu)
+     * @param iconName the filename of the icon to use
+     * @param tooltip  a longer description of the action that will be displayed in the tooltip. Please note
+     *           that html is not supported for menu actions on some platforms.
+     * @param shortcut a ready-created shortcut object or null if you don't want a shortcut. But you always
+     *            do want a shortcut, remember you can always register it with group=none, so you
+     *            won't be assigned a shortcut unless the user configures one. If you pass null here,
+     *            the user CANNOT configure a shortcut for your action.
+     * @param register register this action for the toolbar preferences?
+     * @param toolbar identifier for the toolbar preferences. The iconName is used, if this parameter is null
+     * @param installAdapters false, if you don't want to install layer changed and selection changed adapters
+     */
     public AbstractMergeAction(String name, String iconName, String tooltip, Shortcut shortcut,
     boolean register, String toolbar, boolean installAdapters) {
@@ -59 +79 @@
     }
 
+    /**
+     * Ask user to choose the target layer.
+     * @param targetLayers list of candidate target layers.
+     * @return the chosen layer
+     */
     protected static Layer askTargetLayer(List<Layer> targetLayers) {
         return askTargetLayer(targetLayers.toArray(new Layer[targetLayers.size()]),
@@ -99 +124 @@
     }
 
+    /**
+     * Warns user when there no layers the source layer could be merged to.
+     * @param sourceLayer source layer
+     */
     protected void warnNoTargetLayersForSourceLayer(Layer sourceLayer) {
-        JOptionPane.showMessageDialog(Main.parent,
-                tr("<html>There are no layers the source layer<br>''{0}''<br>could be merged to.</html>",
-                        Utils.escapeReservedCharactersHTML(sourceLayer.getName())),
-                tr("No target layers"), JOptionPane.WARNING_MESSAGE);
+        String message = tr("<html>There are no layers the source layer<br>''{0}''<br>could be merged to.</html>",
+                Utils.escapeReservedCharactersHTML(sourceLayer.getName()));
+        if (!GraphicsEnvironment.isHeadless()) {
+            JOptionPane.showMessageDialog(Main.parent, message, tr("No target layers"), JOptionPane.WARNING_MESSAGE);
+        }
     }
 }

trunk/src/org/openstreetmap/josm/actions/MergeLayerAction.java

@@ -10 +10 @@
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.Future;
 
 import org.openstreetmap.josm.Main;
@@ -38 +39 @@
     }
 
-    protected void doMerge(List<Layer> targetLayers, final Collection<Layer> sourceLayers) {
+    /**
+     * Submits merge of layers.
+     * @param targetLayers possible target layers
+     * @param sourceLayers source layers
+     * @return a Future representing pending completion of the merge task, or {@code null}
+     * @since 11885 (return type)
+     */
+    protected Future<?> doMerge(List<Layer> targetLayers, final Collection<Layer> sourceLayers) {
         final Layer targetLayer = askTargetLayer(targetLayers);
         if (targetLayer == null)
-            return;
+            return null;
         final Object actionName = getValue(NAME);
-        Main.worker.submit(() -> {
+        return Main.worker.submit(() -> {
                 final long start = System.currentTimeMillis();
                 boolean layerMerged = false;
@@ -69 +77 @@
      * Merges a list of layers together.
      * @param sourceLayers The layers to merge
+     * @return a Future representing pending completion of the merge task, or {@code null}
+     * @since 11885 (return type)
      */
-    public void merge(List<Layer> sourceLayers) {
-        doMerge(sourceLayers, sourceLayers);
+    public Future<?> merge(List<Layer> sourceLayers) {
+        return doMerge(sourceLayers, sourceLayers);
     }
 
@@ -77 +87 @@
      * Merges the given source layer with another one, determined at runtime.
      * @param sourceLayer The source layer to merge
+     * @return a Future representing pending completion of the merge task, or {@code null}
+     * @since 11885 (return type)
      */
-    public void merge(Layer sourceLayer) {
+    public Future<?> merge(Layer sourceLayer) {
         if (sourceLayer == null)
-            return;
+            return null;
         List<Layer> targetLayers = LayerListDialog.getInstance().getModel().getPossibleMergeTargets(sourceLayer);
         if (targetLayers.isEmpty()) {
             warnNoTargetLayersForSourceLayer(sourceLayer);
-            return;
+            return null;
         }
-        doMerge(targetLayers, Collections.singleton(sourceLayer));
+        return doMerge(targetLayers, Collections.singleton(sourceLayer));
     }
 
@@ -112 +124 @@
     }
 
+    /**
+     * Returns the source layer.
+     * @return the source layer
+     */
     protected Layer getSourceLayer() {
-        return Main.map != null ? Main.getLayerManager().getActiveLayer() : null;
+        return Main.getLayerManager().getActiveLayer();
     }
 

trunk/src/org/openstreetmap/josm/gui/MapFrame.java

@@ -243 +243 @@
 
         // toolBarToggles, toggle dialog buttons
-        LayerListDialog.createInstance(this);
+        LayerListDialog.createInstance(mapView.getLayerManager());
         propertiesDialog = new PropertiesDialog();
         selectionListDialog = new SelectionListDialog();

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

@@ -44 +44 @@
 import org.openstreetmap.josm.actions.MergeLayerAction;
 import org.openstreetmap.josm.data.preferences.AbstractProperty;
-import org.openstreetmap.josm.gui.MapFrame;
 import org.openstreetmap.josm.gui.MapView;
 import org.openstreetmap.josm.gui.SideButton;
@@ -90 +89 @@
 
     /**
-     * Creates the instance of the dialog. It's connected to the map frame <code>mapFrame</code>
+     * Creates the instance of the dialog. It's connected to the layer manager
      *
-     * @param mapFrame the map frame
-     */
-    public static void createInstance(MapFrame mapFrame) {
+     * @param layerManager the layer manager
+     * @since 11885 (signature)
+     */
+    public static void createInstance(MainLayerManager layerManager) {
         if (instance != null)
             throw new IllegalStateException("Dialog was already created");
-        instance = new LayerListDialog(mapFrame);
+        instance = new LayerListDialog(layerManager);
     }
 
@@ -105 +105 @@
      * @return the instance of the dialog
      * @throws IllegalStateException if the dialog is not created yet
-     * @see #createInstance(MapFrame)
+     * @see #createInstance(MainLayerManager)
      */
     public static LayerListDialog getInstance() {
@@ -164 +164 @@
 
     /**
-     * Creates a layer list and attach it to the given mapView.
-     * @param mapFrame map frame
-     */
-    protected LayerListDialog(MapFrame mapFrame) {
-        this(mapFrame.mapView.getLayerManager());
-    }
-
-    /**
-     * Creates a layer list and attach it to the given mapView.
+     * Creates a layer list and attach it to the given layer manager.
      * @param layerManager The layer manager this list is for
      * @since 10467

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

@@ -265 +265 @@
      */
     protected void registerInWindowMenu() {
-        windowMenuItem = MainMenu.addWithCheckbox(Main.main.menu.windowMenu,
-                (JosmAction) getToggleAction(),
-                MainMenu.WINDOW_MENU_GROUP.TOGGLE_DIALOG);
+        if (Main.main != null) {
+            windowMenuItem = MainMenu.addWithCheckbox(Main.main.menu.windowMenu,
+                    (JosmAction) getToggleAction(),
+                    MainMenu.WINDOW_MENU_GROUP.TOGGLE_DIALOG);
+        }
     }
 
@@ -329 +331 @@
         // toggling the selected value in order to enforce PropertyChangeEvents
         setIsShowing(true);
-        windowMenuItem.setState(true);
+        if (windowMenuItem != null) {
+            windowMenuItem.setState(true);
+        }
         toggleAction.putValue("selected", Boolean.FALSE);
         toggleAction.putValue("selected", Boolean.TRUE);
@@ -374 +378 @@
         closeDetachedDialog();
         this.setVisible(false);
-        windowMenuItem.setState(false);
+        if (windowMenuItem != null) {
+            windowMenuItem.setState(false);
+        }
         setIsShowing(false);
         toggleAction.putValue("selected", Boolean.FALSE);
@@ -456 +462 @@
             hideNotify();
         }
-        Main.main.menu.windowMenu.remove(windowMenuItem);
+        if (Main.main != null) {
+            Main.main.menu.windowMenu.remove(windowMenuItem);
+        }
         Toolkit.getDefaultToolkit().removeAWTEventListener(this);
         Main.pref.removePreferenceChangeListener(this);