Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@
* Contributors:
* IBM Corporation - initial API and implementation
* Alexander Fedorov <[email protected]> - Bug 548428
* Dinesh Palanisamy (ETAS GmbH) - Issue 1530
*******************************************************************************/
package org.eclipse.ui.actions;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;

import org.eclipse.jface.action.ActionContributionItem;
import org.eclipse.jface.action.IAction;
Expand All @@ -28,6 +33,7 @@
import org.eclipse.ui.activities.WorkbenchActivityHelper;
import org.eclipse.ui.internal.WorkbenchPlugin;
import org.eclipse.ui.internal.actions.NewWizardShortcutAction;
import org.eclipse.ui.internal.dialogs.DynamicMenuSelection;
import org.eclipse.ui.internal.dialogs.WorkbenchWizardElement;
import org.eclipse.ui.internal.registry.WizardsRegistryReader;
import org.eclipse.ui.wizards.IWizardCategory;
Expand Down Expand Up @@ -186,9 +192,29 @@ protected void addItems(List<IContributionItem> list) {
list.add(new ActionContributionItem(newExampleAction));
list.add(new Separator());
}

// To add shortcuts from OTHER... wizard regardless of perspective
Collection<IContributionItem> otherItems = new LinkedList<>();
if (!DynamicMenuSelection.getInstance().getSelectedFromOther().isEmpty()) {
for (String selectedItemsFormOthers : DynamicMenuSelection.getInstance().getSelectedFromOther()) {
IAction action = getAction(selectedItemsFormOthers);
otherItems.add(new ActionContributionItem(action));
}
dynamicMenuCheck(list, otherItems);
}
list.add(new ActionContributionItem(getShowDialogAction()));
}

private void dynamicMenuCheck(List<IContributionItem> list, Collection<IContributionItem> otherItems) {
Set<IContributionItem> existingShortcutsInPerspective = new HashSet<>(list);
for (IContributionItem item : otherItems) {
if (!existingShortcutsInPerspective.contains(item)) {
list.add(item);
existingShortcutsInPerspective.add(item);
}
}
}

private boolean isNewProjectWizardAction(IAction action) {
if (action instanceof NewWizardShortcutAction) {
IWizardDescriptor wizardDescriptor= ((NewWizardShortcutAction) action).getWizardDescriptor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,10 @@ public void dispose() {
/*
* Returns the action for the given wizard id, or null if not found.
*/
private IAction getAction(String id) {
/**
* @since 3.132
*/
Comment on lines +169 to +171
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please merged with already existing Javadoc block.

protected IAction getAction(String id) {
// Keep a cache, rather than creating a new action each time,
// so that image caching in ActionContributionItem works.
IAction action = actions.get(id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IConfigurationElement;
import org.eclipse.core.runtime.IExtensionPoint;
Expand Down Expand Up @@ -53,6 +54,7 @@
import org.eclipse.ui.IWorkingSetManager;
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.internal.decorators.DecoratorManager;
import org.eclipse.ui.internal.dialogs.DynamicMenuSelection;
import org.eclipse.ui.internal.dialogs.WorkbenchPreferenceManager;
import org.eclipse.ui.internal.help.CommandHelpServiceImpl;
import org.eclipse.ui.internal.help.HelpServiceImpl;
Expand Down Expand Up @@ -80,6 +82,7 @@
import org.eclipse.ui.testing.TestableObject;
import org.eclipse.ui.views.IViewRegistry;
import org.eclipse.ui.wizards.IWizardRegistry;
import org.eclipse.ui.workbench.DynamicMenuItem;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.BundleEvent;
Expand Down Expand Up @@ -202,6 +205,8 @@ public class WorkbenchPlugin extends AbstractUIPlugin {

private ICommandHelpService commandHelpService;

private DynamicMenuItem dynamicMenuItem;

/**
* Create an instance of the WorkbenchPlugin. The workbench plugin is
* effectively the "application" for the workbench UI. The entire UI operates as
Expand All @@ -210,6 +215,7 @@ public class WorkbenchPlugin extends AbstractUIPlugin {
public WorkbenchPlugin() {
super();
inst = this;
dynamicMenuItem = new DynamicMenuItem();
}

/**
Expand Down Expand Up @@ -766,6 +772,10 @@ public void start(BundleContext context) throws Exception {
// to be loaded.s
if (uiBundle != null)
uiBundle.start(Bundle.START_TRANSIENT);

// For dynamic menu items
Set<String> selectedFromOther = DynamicMenuSelection.getInstance().getSelectedFromOther();
selectedFromOther.addAll(dynamicMenuItem.getDynamicMenuItems());
Comment on lines +777 to +778
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do this, this is a bad practice. If you want to add several items to that list then add a proper method in DynamicMenuSelection and document it.

} catch (BundleException e) {
WorkbenchPlugin.log("Unable to load UI activator", e); //$NON-NLS-1$
}
Expand Down Expand Up @@ -1048,6 +1058,9 @@ public void stop(BundleContext context) throws Exception {
testableTracker.close();
testableTracker = null;
}
// For dynamic menu items
Set<String> selectedFromOther = DynamicMenuSelection.getInstance().getSelectedFromOther();
dynamicMenuItem.setDynamicMenuItems(selectedFromOther);
super.stop(context);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*******************************************************************************
* Copyright (c) 2023 ETAS GmbH and others, all rights reserved.
*
* 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:
* ETAS GmbH - initial API and implementation
*******************************************************************************/

package org.eclipse.ui.internal.dialogs;

import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.Set;

/**
* @since 3.5
*
*/
public class DynamicMenuSelection {
private Set<String> selectedFromOther = new LinkedHashSet<>();
private static DynamicMenuSelection instance;
private static final int MAX_MENU_SIZE = 5;

public static DynamicMenuSelection getInstance() {
synchronized (DynamicMenuSelection.class) {
if (instance == null) {
instance = new DynamicMenuSelection();
}
return instance;
}
}

public void addItems(String shortcuts) {
selectedFromOther.add(shortcuts);
if (selectedFromOther.size() > MAX_MENU_SIZE) {
Iterator<String> iterator = selectedFromOther.iterator();
iterator.next();
iterator.remove();
}
}

/**
* @return Returns the selectedFromOther.
*/
public Set<String> getSelectedFromOther() {
return selectedFromOther;
}
Comment on lines +51 to +53
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please return an immutable set to avoid calls like the one you used in WorkbenchPlugin.start(). Adding stuff to this set should be done from within this class, otherwise it becomes really difficult to track who adds what.


}
Original file line number Diff line number Diff line change
Expand Up @@ -689,4 +689,11 @@ public IWorkbenchWizard createWizard() throws CoreException {

updateDescription(selectedObject);
}

/**
* @return Returns the selectedElement.
*/
public IWizardDescriptor getSelectedElement() {
return selectedElement;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ public void createControl(Composite parent) {
* they will persist into the next invocation of this wizard page
*/
protected void saveWidgetValues() {
// For dynamic menu
DynamicMenuSelection.getInstance().addItems(newResourcePage.getSelectedElement().getId());
newResourcePage.saveWidgetValues();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*******************************************************************************
* Copyright (c) 2023 ETAS GmbH and others, all rights reserved.
*
* 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:
* ETAS GmbH - initial API and implementation
*******************************************************************************/

package org.eclipse.ui.workbench;

import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import org.eclipse.core.runtime.preferences.IEclipsePreferences;
import org.eclipse.core.runtime.preferences.InstanceScope;

public class DynamicMenuItem {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some JavaDoc to the class and also to the public methods.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is to become a new API class, then it requires proper Javadoc and @since ... markers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it would be even better if this could remain internal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, DynamicMenuItem is not a great name, especially if it's meant to serve only the newWizard case. I think it would be better to have this nested in the NewWizardNewPage or something, with more narrowly-scope names for preferences.
So it could be invoked with something like NewWizardNewPage.getItemsFromPreferences() or something of that sort that reads more efficient.

Copy link
Contributor

@lathapatil lathapatil May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mickaelistria This cannot be nested in NewWizardNewPage class as it is having default / Package visibility . Can I use NewWizard class instead ? (Need to access from WorkbenchPlugin.java )


private static final String PLUGIN_ID = "org.eclipse.ui.workbench"; //$NON-NLS-1$
private static final String DYNAMIC_MENU_ITEMS = "dynamicMenuItems"; //$NON-NLS-1$

public List<String> getDynamicMenuItems() {
IEclipsePreferences pref = InstanceScope.INSTANCE.getNode(PLUGIN_ID);
String jsn = pref.get(DYNAMIC_MENU_ITEMS, null);
if (jsn != null) {
Gson gsn = new Gson();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GSon is a relatively heavy library to use for this case. Can you try avoiding it, for example by storing/reading the list as comma-separated ids and use pref.get(DYNAMIC_MENU_ITEM, "").split(",") ?

Type typ = new TypeToken<List<String>>() {
}.getType();
return gsn.fromJson(jsn, typ);
}
return new ArrayList<>();
}

public void setDynamicMenuItems(Set<String> set) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this parameter to reflect what it represents and not what it is. One can see this is a set by looking at the type but one doesn't know what the set contains (I bet items would be more suitable)

IEclipsePreferences pref = InstanceScope.INSTANCE.getNode(PLUGIN_ID);
Gson gsn = new Gson();
String jsn = gsn.toJson(set);
pref.put(DYNAMIC_MENU_ITEMS, jsn);
try {
pref.flush();
} catch (org.osgi.service.prefs.BackingStoreException e) {
e.printStackTrace();
}
}

}
3 changes: 2 additions & 1 deletion bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)",
org.eclipse.e4.ui.workbench.addons.swt;bundle-version="0.10.0",
org.eclipse.e4.ui.services;bundle-version="1.3.0",
org.eclipse.emf.ecore.xmi;bundle-version="2.11.0",
org.eclipse.e4.core.di.extensions;bundle-version="0.13.0"
org.eclipse.e4.core.di.extensions;bundle-version="0.13.0",
com.google.gson;bundle-version="2.10.1"
Import-Package: com.ibm.icu.util,
jakarta.annotation;version="[2.1.0,3.0.0)",
jakarta.inject;version="[2.0.0,3.0.0)",
Expand Down