Index: /trunk/src/org/openstreetmap/josm/actions/AbstractMergeAction.java =================================================================== --- /trunk/src/org/openstreetmap/josm/actions/AbstractMergeAction.java (revision 11884) +++ /trunk/src/org/openstreetmap/josm/actions/AbstractMergeAction.java (revision 11885) @@ -10,5 +10,4 @@ import javax.swing.DefaultListCellRenderer; -import javax.swing.Icon; import javax.swing.JLabel; import javax.swing.JList; @@ -24,9 +23,12 @@ 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,6 +38,5 @@ 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,13 +46,32 @@ /** * 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,4 +79,9 @@ } + /** + * Ask user to choose the target layer. + * @param targetLayers list of candidate target layers. + * @return the chosen layer + */ protected static Layer askTargetLayer(List targetLayers) { return askTargetLayer(targetLayers.toArray(new Layer[targetLayers.size()]), @@ -99,9 +124,14 @@ } + /** + * 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("There are no layers the source layer
''{0}''
could be merged to.", - Utils.escapeReservedCharactersHTML(sourceLayer.getName())), - tr("No target layers"), JOptionPane.WARNING_MESSAGE); + String message = tr("There are no layers the source layer
''{0}''
could be merged to.", + Utils.escapeReservedCharactersHTML(sourceLayer.getName())); + if (!GraphicsEnvironment.isHeadless()) { + JOptionPane.showMessageDialog(Main.parent, message, tr("No target layers"), JOptionPane.WARNING_MESSAGE); + } } } Index: /trunk/src/org/openstreetmap/josm/actions/MergeLayerAction.java =================================================================== --- /trunk/src/org/openstreetmap/josm/actions/MergeLayerAction.java (revision 11884) +++ /trunk/src/org/openstreetmap/josm/actions/MergeLayerAction.java (revision 11885) @@ -10,4 +10,5 @@ import java.util.Collections; import java.util.List; +import java.util.concurrent.Future; import org.openstreetmap.josm.Main; @@ -38,10 +39,17 @@ } - protected void doMerge(List targetLayers, final Collection 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 targetLayers, final Collection 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,7 +77,9 @@ * 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 sourceLayers) { - doMerge(sourceLayers, sourceLayers); + public Future merge(List sourceLayers) { + return doMerge(sourceLayers, sourceLayers); } @@ -77,14 +87,16 @@ * 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 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,6 +124,10 @@ } + /** + * Returns the source layer. + * @return the source layer + */ protected Layer getSourceLayer() { - return Main.map != null ? Main.getLayerManager().getActiveLayer() : null; + return Main.getLayerManager().getActiveLayer(); } Index: /trunk/src/org/openstreetmap/josm/gui/MapFrame.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/MapFrame.java (revision 11884) +++ /trunk/src/org/openstreetmap/josm/gui/MapFrame.java (revision 11885) @@ -243,5 +243,5 @@ // toolBarToggles, toggle dialog buttons - LayerListDialog.createInstance(this); + LayerListDialog.createInstance(mapView.getLayerManager()); propertiesDialog = new PropertiesDialog(); selectionListDialog = new SelectionListDialog(); Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/LayerListDialog.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/LayerListDialog.java (revision 11884) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/LayerListDialog.java (revision 11885) @@ -44,5 +44,4 @@ 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,12 +89,13 @@ /** - * Creates the instance of the dialog. It's connected to the map frame mapFrame + * 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,5 +105,5 @@ * @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,13 +164,5 @@ /** - * 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 Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/ToggleDialog.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/ToggleDialog.java (revision 11884) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/ToggleDialog.java (revision 11885) @@ -265,7 +265,9 @@ */ 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,5 +331,7 @@ // 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,5 +378,7 @@ closeDetachedDialog(); this.setVisible(false); - windowMenuItem.setState(false); + if (windowMenuItem != null) { + windowMenuItem.setState(false); + } setIsShowing(false); toggleAction.putValue("selected", Boolean.FALSE); @@ -456,5 +462,7 @@ 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); Index: /trunk/test/unit/org/openstreetmap/josm/actions/MergeLayerActionTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/actions/MergeLayerActionTest.java (revision 11885) +++ /trunk/test/unit/org/openstreetmap/josm/actions/MergeLayerActionTest.java (revision 11885) @@ -0,0 +1,81 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.actions; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.gui.layer.LayerManagerTest.TestLayer; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.testutils.JOSMTestRules; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +/** + * Unit tests for class {@link MergeLayerAction}. + */ +public class MergeLayerActionTest { + + /** + * Setup test. + */ + @Rule + @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public JOSMTestRules test = new JOSMTestRules().platform().commands(); + + private MergeLayerAction action; + + /** + * Setup test. + */ + @Before + public void setUp() { + if (action == null) { + action = new MergeLayerAction(); + } + for (TestLayer testLayer : Main.getLayerManager().getLayersOfType(TestLayer.class)) { + Main.getLayerManager().removeLayer(testLayer); + } + } + + /** + * Tests that no error occurs when no source layer exists. + */ + @Test + public void testMergeNoSourceLayer() { + assertNull(Main.getLayerManager().getActiveLayer()); + action.actionPerformed(null); + assertEquals(0, Main.getLayerManager().getLayers().size()); + } + + /** + * Tests that no error occurs when no target layer exists. + */ + @Test + public void testMergeNoTargetLayer() { + OsmDataLayer layer = new OsmDataLayer(new DataSet(), "", null); + Main.getLayerManager().addLayer(layer); + assertEquals(1, Main.getLayerManager().getLayers().size()); + assertNull(action.merge(layer)); + assertEquals(1, Main.getLayerManager().getLayers().size()); + } + + /** + * Tests that the merge is done with two empty layers. + * @throws Exception if any error occurs + */ + @Test + public void testMergeTwoEmptyLayers() throws Exception { + OsmDataLayer layer1 = new OsmDataLayer(new DataSet(), "1", null); + OsmDataLayer layer2 = new OsmDataLayer(new DataSet(), "2", null); + Main.getLayerManager().addLayer(layer1); + Main.getLayerManager().addLayer(layer2); + assertEquals(2, Main.getLayerManager().getLayers().size()); + action.merge(layer2).get(); + assertEquals(1, Main.getLayerManager().getLayers().size()); + } +}