From 4a918e9bab3fddbe2062edcd761eecb379189930 Mon Sep 17 00:00:00 2001 From: Jozef Tomek Date: Sat, 29 Jun 2024 09:20:51 +0200 Subject: [PATCH] Simplify populating enhancements menu & colors sub-menu Re-create them every time they're being requested. Fixes #1442 --- .../MenuVisibilityMenuItemsConfigurer.java | 89 ------------------- .../SignatureStylingColorSubMenuItem.java | 51 ++++------- .../SignatureStylingMenuToolbarAction.java | 51 +++-------- 3 files changed, 28 insertions(+), 163 deletions(-) delete mode 100644 org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/MenuVisibilityMenuItemsConfigurer.java diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/MenuVisibilityMenuItemsConfigurer.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/MenuVisibilityMenuItemsConfigurer.java deleted file mode 100644 index 90cbf7796be..00000000000 --- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/MenuVisibilityMenuItemsConfigurer.java +++ /dev/null @@ -1,89 +0,0 @@ -/******************************************************************************* -* Copyright (c) 2024 Jozef Tomek and others. -* -* This program and the accompanying materials -* are made available under the terms of the Eclipse Public License 2.0 -* which accompanies this distribution, and is available at -* https://www.eclipse.org/legal/epl-2.0/ -* -* SPDX-License-Identifier: EPL-2.0 -* -* Contributors: -* Jozef Tomek - initial API and implementation -*******************************************************************************/ -package org.eclipse.jdt.internal.ui.viewsupport; - -import java.util.function.BiConsumer; - -import org.eclipse.swt.events.MenuEvent; -import org.eclipse.swt.events.MenuListener; -import org.eclipse.swt.widgets.Menu; -import org.eclipse.swt.widgets.MenuItem; - -import org.eclipse.jface.action.ActionContributionItem; - -/** - * Facility to make it possible for menu widget items to react to their parent menu's visibility events. - * Menu items become receivers of these events by means of being created from {@link ActionContributionItem} - * that wraps and action implementing convenient callback interface {@link IMenuVisibilityMenuItemAction}. - */ -public final class MenuVisibilityMenuItemsConfigurer { - - /** - * Does necessary setup of the menu widget to enable forwarding menu's visibility events to all of it's - * child menu items that are {@link IMenuVisibilityMenuItemAction receivers} of these events. - *

- * Attention: This method must be called when all menu item widgets were already created inside the menu. - * @param menu widget containing events receiving items - */ - public static void registerForMenu(Menu menu) { - menu.addMenuListener(new MenuListenerImpl(menu)); - } - - private static class MenuListenerImpl implements MenuListener { - final Menu menu; - - private MenuListenerImpl(Menu menu) { - this.menu= menu; - } - - @Override - public void menuShown(MenuEvent e) { - handleEvent(e, IMenuVisibilityMenuItemAction::menuShown); - } - - @Override - public void menuHidden(MenuEvent e) { - handleEvent(e, IMenuVisibilityMenuItemAction::menuHidden); - } - - private void handleEvent(MenuEvent e, BiConsumer callback) { - for (MenuItem item : menu.getItems()) { - if (item.getData() instanceof ActionContributionItem actionItem - && actionItem.getAction() instanceof IMenuVisibilityMenuItemAction listener) { - callback.accept(listener, e); - } - } - } - } - - /** - * Menu visibility listener tagging interface for menu item actions. If a menu item was created from {@link ActionContributionItem} - * and the action it is wrapping implements this interface, then action's methods defined in this interface are called - * on change of menu visibility. - */ - public interface IMenuVisibilityMenuItemAction { - - /** - * Notification when the menu, that contains menu item wrapping an action that is target of this call, was shown. - * @param event event forwarded from menu's {@link MenuListener} - */ - default void menuShown(MenuEvent event) {} - - /** - * Notification when the menu, that contains menu item wrapping an action that is target of this call, was hidden. - * @param event event forwarded from menu's {@link MenuListener} - */ - default void menuHidden(MenuEvent event) {} - } -} \ No newline at end of file diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingColorSubMenuItem.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingColorSubMenuItem.java index cb9421de6b5..7255f62a8a3 100644 --- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingColorSubMenuItem.java +++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingColorSubMenuItem.java @@ -13,12 +13,9 @@ *******************************************************************************/ package org.eclipse.jdt.internal.ui.viewsupport.javadoc; -import java.util.NoSuchElementException; import java.util.Objects; import java.util.function.Supplier; -import java.util.stream.Stream; -import org.eclipse.swt.events.MenuEvent; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Menu; import org.eclipse.swt.widgets.Shell; @@ -31,12 +28,11 @@ import org.eclipse.jdt.internal.corext.util.Messages; import org.eclipse.jdt.internal.ui.viewsupport.JavaElementLinks; -import org.eclipse.jdt.internal.ui.viewsupport.MenuVisibilityMenuItemsConfigurer.IMenuVisibilityMenuItemAction; /** - * Menu item action for building & presenting color preferences sub-menu of javadoc styling menu. + * Menu item action for building & presenting color preferences sub-menu of javadoc styling menu. */ -class SignatureStylingColorSubMenuItem extends Action implements IMenuCreator, IMenuVisibilityMenuItemAction { +class SignatureStylingColorSubMenuItem extends Action implements IMenuCreator { private final Shell parentShell; private final Supplier javadocContentSupplier; @@ -51,25 +47,28 @@ public SignatureStylingColorSubMenuItem(Shell parent, Supplier javadocCo @Override public Menu getMenu(Menu parent) { + // we keep it simple here and just re-create new menu with correct items + dispose(); var content= javadocContentSupplier.get(); - if (menu == null && content != null) { + if (content != null) { menu= new Menu(parent); new ActionContributionItem(new ResetSignatureStylingColorsPreferencesMenuItem()).fill(menu, -1); new Separator().fill(menu, -1); int typeParamsReferencesCount= JavaElementLinks.getNumberOfTypeParamsReferences(content); - for (int i= 1; i <= typeParamsReferencesCount; i++) { - var item= new ActionContributionItem(new SignatureStylingColorPreferenceMenuItem( - parentShell, - JavadocStylingMessages.JavadocStyling_colorPreferences_typeParameter, - i, - JavaElementLinks::getColorPreferenceForTypeParamsReference, - JavaElementLinks::setColorPreferenceForTypeParamsReference)); - item.fill(menu, -1); - } if (typeParamsReferencesCount == 0) { new ActionContributionItem(new NoSignatureStylingTypeParametersMenuItem()).fill(menu, -1); + } else { + for (int i= 1; i <= typeParamsReferencesCount; i++) { + var item= new ActionContributionItem(new SignatureStylingColorPreferenceMenuItem( + parentShell, + JavadocStylingMessages.JavadocStyling_colorPreferences_typeParameter, + i, + JavaElementLinks::getColorPreferenceForTypeParamsReference, + JavaElementLinks::setColorPreferenceForTypeParamsReference)); + item.fill(menu, -1); + } } var typeParamsReferenceIndices= JavaElementLinks.getColorPreferencesIndicesForTypeParamsReference(); @@ -102,26 +101,6 @@ public void dispose() { } } - @Override - public void menuShown(MenuEvent e) { - if (menu != null) { - var parentMenu= ((Menu) e.widget); - // jface creates & displays proxies for sub-menus so just modifying items in sub-menu we return won't work, but we have to remove whole sub-menu item from menu - var menuItem= Stream.of(parentMenu.getItems()) - .filter(mi -> mi.getData() instanceof ActionContributionItem aci && aci.getAction() == this) - .findFirst().orElseThrow(() -> new NoSuchElementException( - "This " + //$NON-NLS-1$ - SignatureStylingColorSubMenuItem.class.getSimpleName() - + " instance not found inside menu being shown")); //$NON-NLS-1$ - // can't be done in menuHidden() since SWT.Selection is fired after SWT.Hide, thus run() action would not be executed since item would be disposed - menuItem.dispose(); - - // re-add the sub-mebu as new menu item - var item= new ActionContributionItem(this); - item.fill(parentMenu, -1); - } - } - private static final class ResetSignatureStylingColorsPreferencesMenuItem extends Action { public ResetSignatureStylingColorsPreferencesMenuItem() { super(JavadocStylingMessages.JavadocStyling_colorPreferences_resetAll); diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingMenuToolbarAction.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingMenuToolbarAction.java index f7c049546b2..ced6c39b79c 100644 --- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingMenuToolbarAction.java +++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingMenuToolbarAction.java @@ -17,11 +17,9 @@ import java.util.function.Supplier; import java.util.stream.Stream; -import org.eclipse.swt.events.MenuEvent; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Event; import org.eclipse.swt.widgets.Menu; -import org.eclipse.swt.widgets.MenuItem; import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.ToolBar; @@ -33,8 +31,6 @@ import org.eclipse.jdt.internal.ui.JavaPluginImages; import org.eclipse.jdt.internal.ui.viewsupport.JavaElementLinks; import org.eclipse.jdt.internal.ui.viewsupport.JavaElementLinks.IStylingConfigurationListener; -import org.eclipse.jdt.internal.ui.viewsupport.MenuVisibilityMenuItemsConfigurer; -import org.eclipse.jdt.internal.ui.viewsupport.MenuVisibilityMenuItemsConfigurer.IMenuVisibilityMenuItemAction; import org.eclipse.jdt.internal.ui.viewsupport.browser.BrowserTextAccessor; import org.eclipse.jdt.internal.ui.viewsupport.browser.BrowserTextAccessor.IBrowserContentChangeListener; @@ -61,7 +57,6 @@ public SignatureStylingMenuToolbarAction(Shell parent, BrowserTextAccessor brows Shell topLevelShell = (parent.getParent() instanceof Shell parentShell) ? parentShell : parent; enabledActions= new Action[] { new ToggleSignatureTypeParametersColoringAction(), - // widget for following action is being removed and re-added repeatedly, see SignatureStylingColorSubMenuItem.menuShown() new SignatureStylingColorSubMenuItem(topLevelShell, javadocContentSupplier)}; actions= noStylingActions; setMenuCreator(this); @@ -81,29 +76,18 @@ public void browserContentChanged(Supplier contentAccessor) { } var content= contentAccessor.get(); if (content != null && !content.isBlank() && JavaElementLinks.isStylingPresent(content)) { - reAddActionItems(enabledActions); + actions= enabledActions; } else { - reAddActionItems(noStylingActions); - } - } - - private void reAddActionItems(Action[] newActions) { - if (actions != newActions) { - actions= newActions; - if (menu != null) { - Stream.of(menu.getItems()).forEach(MenuItem::dispose); - addMenuItems(); - } + actions= noStylingActions; } } @Override public Menu getMenu(Control p) { - if (menu == null) { - menu= new Menu(parent); - addMenuItems(); - MenuVisibilityMenuItemsConfigurer.registerForMenu(menu); - } + // we keep it simple here and just re-create new menu with correct items + dispose(); + menu= new Menu(parent); + Stream.of(actions).forEach(action -> new ActionContributionItem(action).fill(menu, -1)); return menu; } @@ -112,14 +96,11 @@ public Menu getMenu(Menu p) { return null; } - private void addMenuItems() { - Stream.of(actions).forEach(action -> new ActionContributionItem(action).fill(menu, -1)); - } - @Override public void dispose() { if (menu != null) { menu.dispose(); + menu= null; } } @@ -147,14 +128,17 @@ public void stylingStateChanged(boolean isEnabled) { enhancementsEnabled= isEnabled; presentEnhancementsState(); // even if enhancements switched from off to on, only browserContentChanged() sets enabledActions - reAddActionItems(noStylingActions); + actions= noStylingActions; runEnhancementsReconfiguredTask(); }); } @Override public void parametersColoringStateChanged(boolean isEnabled) { - runEnhancementsReconfiguredTask(); + parent.getDisplay().execute(() -> { + enabledActions[0].setChecked(isEnabled); + runEnhancementsReconfiguredTask(); + }); } @Override @@ -172,23 +156,14 @@ public NoStylingEnhancementsAction() { } } - private static class ToggleSignatureTypeParametersColoringAction extends Action implements IMenuVisibilityMenuItemAction { + private static class ToggleSignatureTypeParametersColoringAction extends Action { public ToggleSignatureTypeParametersColoringAction() { super(JavadocStylingMessages.JavadocStyling_typeParamsColoring, IAction.AS_CHECK_BOX); setId(ToggleSignatureTypeParametersColoringAction.class.getSimpleName()); - showCurentPreference(); - } - - private void showCurentPreference() { setChecked(JavaElementLinks.getPreferenceForTypeParamsColoring()); } - @Override - public void menuShown(MenuEvent e) { - showCurentPreference(); - } - @Override public void run() { super.run();