Ticket #22623: 22623.2.patch

src/org/openstreetmap/josm/plugins/buildings_tools/AdvancedSettingsDialog.java

@@ -19 +19 @@
     private final JCheckBox cBigMode = new JCheckBox(tr("Big buildings mode"));
     private final JCheckBox cSoftCur = new JCheckBox(tr("Rotate crosshair"));
     private final JCheckBox cNoClickDrag = new JCheckBox(tr("Disable click+drag"));
+    private final JCheckBox cToggleMapMode = new JCheckBox(tr("Switch between circle and rectangle modes"));
 
     public AdvancedSettingsDialog() {
         super(tr("Advanced settings"));
@@ -33 +34 @@
         panel.add(cBigMode, GBC.eol().fill(GBC.HORIZONTAL));
         panel.add(cSoftCur, GBC.eol().fill(GBC.HORIZONTAL));
         panel.add(cNoClickDrag, GBC.eol().fill(GBC.HORIZONTAL));
+        panel.add(cToggleMapMode, GBC.eol().fill(GBC.HORIZONTAL));
 
         cBigMode.setSelected(ToolSettings.isBBMode());
         cSoftCur.setSelected(ToolSettings.isSoftCursor());
         cNoClickDrag.setSelected(ToolSettings.isNoClickAndDrag());
+        cToggleMapMode.setSelected(ToolSettings.isTogglingBuildingTypeOnRepeatedKeyPress());
 
+        cToggleMapMode.setToolTipText(tr("This is similar to the select action toggling between lasso and rectangle select modes"));
+
         setupDialog();
         showDialog();
     }
@@ -47 +52 @@
         ToolSettings.setBBMode(cBigMode.isSelected());
         ToolSettings.setSoftCursor(cSoftCur.isSelected());
         ToolSettings.setNoClickAndDrag(cNoClickDrag.isSelected());
+        ToolSettings.setTogglingBuildingTypeOnRepeatedKeyPress(cToggleMapMode.isSelected());
     }
 }

src/org/openstreetmap/josm/plugins/buildings_tools/BuildingsToolsPlugin.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.buildings_tools;
 
+import static org.openstreetmap.josm.tools.I18n.tr;
+
 import javax.swing.JMenu;
 
 import org.openstreetmap.josm.data.coor.EastNorth;
@@ -14 +16 @@
 import org.openstreetmap.josm.gui.MapFrame;
 import org.openstreetmap.josm.plugins.Plugin;
 import org.openstreetmap.josm.plugins.PluginInformation;
+import org.openstreetmap.josm.tools.ImageProvider;
 
+/**
+ * The entry point for the buildings tools plugin
+ */
 public class BuildingsToolsPlugin extends Plugin {
     public static final Projection MERCATOR = Projections.getProjectionByCode("EPSG:3857"); // Mercator
 
@@ -28 +34 @@
 
     public BuildingsToolsPlugin(PluginInformation info) {
         super(info);
-        JMenu dataMenu = MainApplication.getMenu().dataMenu;
-        MainMenu.add(dataMenu, new BuildingSizeAction());
-        MainMenu.add(dataMenu, new BuildingCircleAction());
-        MainMenu.add(dataMenu, new BuildingRectangleAction());
-        MainMenu.add(dataMenu, new MergeAddrPointsAction());
+        JMenu moreToolsMenu = MainApplication.getMenu().moreToolsMenu;
+        if (moreToolsMenu.getMenuComponentCount() > 0) {
+            moreToolsMenu.addSeparator();
+        }
+        MainMenu.add(moreToolsMenu, new DrawBuildingAction());
+        JMenu optionMenu = new JMenu(tr("Draw buildings modes"));
+        optionMenu.setIcon(ImageProvider.get("preference_small", ImageProvider.ImageSizes.MENU));
+        moreToolsMenu.add(optionMenu);
+        MainMenu.add(optionMenu, new BuildingSizeAction());
+        MainMenu.add(optionMenu, new BuildingCircleAction());
+        MainMenu.add(optionMenu, new BuildingRectangleAction());
+        MainMenu.add(optionMenu, new MergeAddrPointsAction());
     }
 
     @Override
     public void mapFrameInitialized(MapFrame oldFrame, MapFrame newFrame) {
         if (oldFrame == null && newFrame != null) {
-            MainApplication.getMap().addMapMode(new IconToggleButton(new DrawBuildingAction()));
+            newFrame.addMapMode(new IconToggleButton(new DrawBuildingAction()));
         }
     }
 }

src/org/openstreetmap/josm/plugins/buildings_tools/DrawBuildingAction.java

@@ -44 +44 @@
 import static org.openstreetmap.josm.tools.I18n.marktr;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+/**
+ * The action for drawing the building
+ */
 public class DrawBuildingAction extends MapMode implements MapViewPaintable, DataSelectionListener,
         KeyPressReleaseListener, ModifierExListener {
+    private static final long serialVersionUID = -3515263157730927711L;
     // We need to avoid opening many file descriptors on Linux under Wayland -- see JOSM #21929. This will probably also
     // improve performance, since we aren't creating cursors all the time.
     private static final Cursor CURSOR_SILO = ImageProvider.getCursor("crosshair", "silo");
@@ -72 +76 @@
 
     private final PreferenceChangedListener shapeChangeListener = event -> updCursor();
 
+    /**
+     * Create a new {@link DrawBuildingAction} object
+     */
     public DrawBuildingAction() {
         super(tr("Draw buildings"), "building", tr("Draw buildings"),
                 Shortcut.registerShortcut("mapmode:buildings",
@@ -193 +200 @@
 
             cancelDrawing();
         }
+        if (!ToolSettings.isTogglingBuildingTypeOnRepeatedKeyPress()
+                || !MainApplication.isDisplayingMapView() || !getShortcut().isEvent(e)) {
+            return;
+        }
+        e.consume();
+        switch (ToolSettings.getShape()) {
+            case CIRCLE:
+                ToolSettings.saveShape(ToolSettings.Shape.RECTANGLE);
+                break;
+            case RECTANGLE:
+                ToolSettings.saveShape(ToolSettings.Shape.CIRCLE);
+        }
     }
 
     @Override

src/org/openstreetmap/josm/plugins/buildings_tools/ToolSettings.java

@@ -133 +133 @@
         Config.getPref().putBoolean("buildings_tools.autoselect_replace_selection", autoSelectReplace);
     }
 
+    /**
+     * Check if we are toggling between {@link Shape} types if the user toggles the mapmode with a keypress
+     * @return {@code true} if we want to change the shape type
+     */
+    public static boolean isTogglingBuildingTypeOnRepeatedKeyPress() {
+        return Config.getPref().getBoolean("buildings_tools.toggle_building_type", false);
+    }
+
+    /**
+     * Set whether or not we are toggling between {@link Shape} types if the user toggles the mapmode with a keypress
+     * @param toggle {@code true} if we want to change the shape type
+     */
+    public static void setTogglingBuildingTypeOnRepeatedKeyPress(boolean toggle) {
+        Config.getPref().putBoolean("buildings_tools.toggle_building_type", toggle);
+    }
+
     public static boolean isNoClickAndDrag() {
         return Config.getPref().getBoolean("buildings_tools.noclickdrag", false);
     }

test/unit/org/openstreetmap/josm/plugins/buildings_tools/BuildingsToolsPluginTest.java

@@ +1 @@
+package org.openstreetmap.josm.plugins.buildings_tools;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.util.jar.Attributes;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.layer.Layer;
+import org.openstreetmap.josm.gui.layer.LayerManager;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.plugins.PluginException;
+import org.openstreetmap.josm.plugins.PluginInformation;
+import org.openstreetmap.josm.testutils.JOSMTestRules;
+import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
+
+/**
+ * Test class for {@link BuildingsToolsPlugin}
+ */
+@BasicPreferences
+class BuildingsToolsPluginTest {
+    @RegisterExtension
+    static JOSMTestRules rule = new JOSMTestRules().projection().main();
+
+    /**
+     * This makes certain we don't have an IAE after removing last layer and adding a new one
+     * @throws PluginException If something occurs during {@link PluginInformation} construction
+     */
+    @Test
+    void testMapReinitialization() throws PluginException {
+        final BuildingsToolsPlugin plugin =
+                new BuildingsToolsPlugin(new PluginInformation(new Attributes(), "buildings_tools", "https://example.com"));
+        MainApplication.getMainPanel().addMapFrameListener(plugin);
+        try {
+            final LayerManager layerManager = MainApplication.getLayerManager();
+            for (int i = 0; i < 20; i++) {
+                final Layer layer = new OsmDataLayer(new DataSet(), "testMapReinitialization", null);
+                assertDoesNotThrow(() -> layerManager.addLayer(layer));
+                assertDoesNotThrow(() -> layerManager.removeLayer(layer));
+            }
+        } finally {
+            MainApplication.getMainPanel().removeMapFrameListener(plugin);
+        }
+    }
+
+}
+ No newline at end of file

test/unit/org/openstreetmap/josm/plugins/buildings_tools/DrawBuildingActionTest.java

@@ +1 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.plugins.buildings_tools;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.awt.event.KeyEvent;
+import java.time.Instant;
+
+import javax.swing.JLabel;
+import javax.swing.KeyStroke;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.testutils.JOSMTestRules;
+import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
+
+/**
+ * Test class for {@link DrawBuildingAction}
+ */
+@BasicPreferences
+class DrawBuildingActionTest {
+    @RegisterExtension
+    static JOSMTestRules rule = new JOSMTestRules().main().projection();
+
+    private static DrawBuildingAction action;
+
+    @BeforeAll
+    static void setup() {
+        action = new DrawBuildingAction();
+    }
+
+    @AfterAll
+    static void tearDown() {
+        action.destroy();
+        action = null;
+    }
+
+
+    /**
+     * Ensure that we are toggling the map mode properly
+     */
+    @ParameterizedTest
+    @ValueSource(booleans = {false, true})
+    void testToggle(boolean setToggle) {
+        // Ensure we are showing the main map
+        MainApplication.getLayerManager().addLayer(new OsmDataLayer(new DataSet(), "testToggle", null));
+        ToolSettings.setTogglingBuildingTypeOnRepeatedKeyPress(setToggle);
+        assertEquals(setToggle, ToolSettings.isTogglingBuildingTypeOnRepeatedKeyPress());
+        final ToolSettings.Shape shape = ToolSettings.getShape();
+        final KeyStroke keyStroke = action.getShortcut().getKeyStroke();
+        MainApplication.getMap().selectMapMode(action);
+        action.doKeyPressed(getKeyEvent(keyStroke));
+        assertNotEquals(setToggle, shape == ToolSettings.getShape());
+        action.doKeyPressed(getKeyEvent(keyStroke));
+        assertEquals(shape, ToolSettings.getShape());
+    }
+
+    /**
+     * Get a key event to send the action
+     * @param keyStroke The keystroke to use
+     * @return The event
+     */
+    private KeyEvent getKeyEvent(KeyStroke keyStroke) {
+        assertNotNull(keyStroke);
+        return new KeyEvent(new JLabel(), KeyEvent.KEY_PRESSED, Instant.now().toEpochMilli(),
+                keyStroke.getModifiers(), keyStroke.getKeyCode(), keyStroke.getKeyChar());
+    }
+}