-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Packages/bundles that are already imported/added are shown #1195
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -18,14 +18,14 @@ | |||||||
|
||||||||
import java.util.Comparator; | ||||||||
import java.util.HashMap; | ||||||||
import java.util.HashSet; | ||||||||
|
||||||||
import org.eclipse.core.runtime.CoreException; | ||||||||
import org.eclipse.core.runtime.IProgressMonitor; | ||||||||
import org.eclipse.core.runtime.IStatus; | ||||||||
import org.eclipse.core.runtime.Status; | ||||||||
import org.eclipse.jface.dialogs.IDialogConstants; | ||||||||
import org.eclipse.jface.dialogs.IDialogSettings; | ||||||||
import org.eclipse.jface.viewers.StructuredSelection; | ||||||||
import org.eclipse.osgi.service.resolver.ExportPackageDescription; | ||||||||
import org.eclipse.pde.core.plugin.IFragment; | ||||||||
import org.eclipse.pde.core.plugin.IFragmentModel; | ||||||||
|
@@ -42,6 +42,7 @@ | |||||||
import org.eclipse.pde.internal.ui.IHelpContextIds; | ||||||||
import org.eclipse.pde.internal.ui.PDEPlugin; | ||||||||
import org.eclipse.pde.internal.ui.PDEUIMessages; | ||||||||
import org.eclipse.swt.widgets.Button; | ||||||||
import org.eclipse.swt.widgets.Composite; | ||||||||
import org.eclipse.swt.widgets.Control; | ||||||||
import org.eclipse.swt.widgets.Shell; | ||||||||
|
@@ -137,6 +138,11 @@ public PluginSelectionDialog(Shell parentShell, IPluginModelBase[] models, boole | |||||||
setListLabelProvider(PDEPlugin.getDefault().getLabelProvider()); | ||||||||
} | ||||||||
|
||||||||
public PluginSelectionDialog(Shell activeWorkbenchShell, IPluginModelBase[] availablePlugins, | ||||||||
boolean multipleSelection, IPluginModelBase model) { | ||||||||
this(activeWorkbenchShell, availablePlugins, multipleSelection); | ||||||||
PDEPlugin.getDefault().getLabelProvider().setCurrentModel(model); | ||||||||
} | ||||||||
@Override | ||||||||
protected void configureShell(Shell newShell) { | ||||||||
super.configureShell(newShell); | ||||||||
|
@@ -153,8 +159,8 @@ private static IPluginModelBase[] getElements(boolean includeFragments) { | |||||||
return PluginRegistry.getActiveModels(includeFragments); | ||||||||
} | ||||||||
|
||||||||
public static HashSet<String> getExistingImports(IPluginModelBase model, boolean includeImportPkg) { | ||||||||
HashSet<String> existingImports = new HashSet<>(); | ||||||||
public static HashMap<String, Boolean> getExistingImports(IPluginModelBase model, boolean includeImportPkg) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use only collection interfaces (as abstract as possible) if possible here and in all other changed code. Using a specific implementation is usually unnecessarily restrictive.
Suggested change
|
||||||||
HashMap<String, Boolean> existingImports = new HashMap<>(); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I oversaw something, but I can only see the key-set of the map being used and values seem not to be considered at all. If that's correct, can this |
||||||||
addSelfAndDirectImports(existingImports, model); | ||||||||
if (model instanceof IFragmentModel) { | ||||||||
IFragment fragment = ((IFragmentModel) model).getFragment(); | ||||||||
|
@@ -169,33 +175,34 @@ public static HashSet<String> getExistingImports(IPluginModelBase model, boolean | |||||||
return existingImports; | ||||||||
} | ||||||||
|
||||||||
private static void addSelfAndDirectImports(HashSet<String> set, IPluginModelBase model) { | ||||||||
private static void addSelfAndDirectImports(HashMap<String, Boolean> existingImports, IPluginModelBase model) { | ||||||||
if (model == null) { | ||||||||
return; | ||||||||
} | ||||||||
set.add(model.getPluginBase().getId()); | ||||||||
existingImports.put(model.getPluginBase().getId(), false); | ||||||||
IPluginImport[] imports = model.getPluginBase().getImports(); | ||||||||
for (IPluginImport pImport : imports) { | ||||||||
String id = pImport.getId(); | ||||||||
if (set.add(id)) { | ||||||||
addReexportedImport(set, id); | ||||||||
} | ||||||||
existingImports.put(id, false); | ||||||||
addReexportedImport(existingImports, id); | ||||||||
|
||||||||
} | ||||||||
} | ||||||||
|
||||||||
private static void addReexportedImport(HashSet<String> set, String id) { | ||||||||
private static void addReexportedImport(HashMap<String, Boolean> existingImports, String id) { | ||||||||
IPluginModelBase model = PluginRegistry.findModel(id); | ||||||||
if (model != null) { | ||||||||
IPluginImport[] imports = model.getPluginBase().getImports(); | ||||||||
for (IPluginImport pImport : imports) { | ||||||||
if (pImport.isReexported() && set.add(pImport.getId())) { | ||||||||
addReexportedImport(set, pImport.getId()); | ||||||||
if (pImport.isReexported()) { | ||||||||
existingImports.put(pImport.getId(), true); | ||||||||
addReexportedImport(existingImports, pImport.getId()); | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
private static void addImportedPackages(IBundlePluginModelBase base, HashSet<String> existingImports) { | ||||||||
private static void addImportedPackages(IBundlePluginModelBase base, HashMap<String, Boolean> existingImports) { | ||||||||
HashMap<String, ImportPackageObject> map = getImportPackages(base); | ||||||||
if (map == null) { | ||||||||
return; | ||||||||
|
@@ -221,7 +228,7 @@ private static void addImportedPackages(IBundlePluginModelBase base, HashSet<Str | |||||||
continue; | ||||||||
} | ||||||||
} | ||||||||
existingImports.add(exported[i].getSupplier().getSymbolicName()); | ||||||||
existingImports.put(exported[i].getSupplier().getSymbolicName(), false); | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
@@ -298,4 +305,22 @@ protected void createButtonsForButtonBar(Composite parent) { | |||||||
createButton(parent, IDialogConstants.CANCEL_ID, IDialogConstants.CANCEL_LABEL, false); | ||||||||
} | ||||||||
|
||||||||
} | ||||||||
@Override | ||||||||
protected void updateButtonsEnableState(IStatus status) { | ||||||||
super.updateButtonsEnableState(status); | ||||||||
Button okButton = getOkButton(); | ||||||||
StructuredSelection currentSelection = super.getSelectedItems(); | ||||||||
HashMap<String, Boolean> existingImports = PluginSelectionDialog | ||||||||
.getExistingImports(PDEPlugin.getDefault().getLabelProvider().getCurrentPluginModel(), false); | ||||||||
if (!currentSelection.isEmpty()) | ||||||||
okButton.setEnabled(false); | ||||||||
for (Object selection : currentSelection) { | ||||||||
if (selection instanceof IPluginModelBase | ||||||||
&& !(existingImports.keySet().contains(((IPluginModelBase) selection).getPluginBase().getId()))) { | ||||||||
Comment on lines
+318
to
+319
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use pattern matching for instanceof in all changed code and if applicable.
Suggested change
Please also reformat the line, not sure if it fits in one line now. |
||||||||
okButton.setEnabled(true); | ||||||||
break; | ||||||||
} | ||||||||
|
||||||||
} | ||||||||
} | ||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of setting
IPluginModelBase currentModel
via a setter and saving it, could you just use the following?Because the less state we have the better. I also don't know if
currentModel
can be null at this point in any usage scenario and if the calledPluginSelectionDialog.getExistingImports
method would handle that well.