Skip to content

Commit

Permalink
Simplify populating enhancements menu & colors sub-menu
Browse files Browse the repository at this point in the history 
Re-create them every time they're being requested.

Fixes eclipse-jdt#1442
  • Loading branch information
@RedeemerSK @iloveeclipse
RedeemerSK authored and iloveeclipse committed Jul 9, 2024
1 parent 3836091 commit 4a918e9
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 163 deletions.
89 changes: 0 additions & 89 deletions ....jdt.ui/ui/org/eclipse/jdt/internal/ui/viewsupport/MenuVisibilityMenuItemsConfigurer.java
View file

This file was deleted.

51 changes: 15 additions & 36 deletions .../ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingColorSubMenuItem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<String> javadocContentSupplier;

Expand All @@ -51,25 +47,28 @@ public SignatureStylingColorSubMenuItem(Shell parent, Supplier<String> 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();
Expand Down Expand Up @@ -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);
Expand Down
51 changes: 13 additions & 38 deletions ...ui/org/eclipse/jdt/internal/ui/viewsupport/javadoc/SignatureStylingMenuToolbarAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand All @@ -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);
Expand All @@ -81,29 +76,18 @@ public void browserContentChanged(Supplier<String> 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;
}

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down

0 comments on commit 4a918e9

Please sign in to comment.