From ece4417f0ca5ca66640866a1670c0fe45ef4614a Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Sat, 6 Jul 2024 14:15:04 +0200 Subject: [PATCH] Always sort BREE header values in ascending Java version order Remove the ability to manually move required execution environments in the Manifest Editor. Instead always sort the values in ascending Java version order. From a technical perspective the order of the values of the 'Bundle-RequiredExecutionEnvironment' header is irrelevant, similar to for example the 'Import-Package' header. Removing the buttons to reorder the values simplifies the UI. Values of existing Plug-in projects are displayed in sorted order regardless of if they are actually sorted in the MANIFEST.MF or not. On the next write all values will be written in sorted order. --- .../pde/internal/core/util/VMUtil.java | 34 ++++++++++++++++--- .../core/text/bundle/BasePackageHeader.java | 4 ++- .../text/bundle/CompositeManifestHeader.java | 12 ++++--- .../RequiredExecutionEnvironmentHeader.java | 3 +- .../bundle/ExecutionEnvironmentTestCase.java | 25 ++++++-------- .../pde/internal/ui/PDEUIMessages.java | 4 --- .../plugin/ExecutionEnvironmentSection.java | 27 ++------------- .../pde/internal/ui/pderesources.properties | 2 -- 8 files changed, 57 insertions(+), 54 deletions(-) diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/VMUtil.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/VMUtil.java index f02d7c243e..9ebeacb0f9 100755 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/VMUtil.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/util/VMUtil.java @@ -15,12 +15,17 @@ package org.eclipse.pde.internal.core.util; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; +import java.util.Map; +import java.util.Properties; +import java.util.stream.Collectors; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; -import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; +import org.eclipse.jdt.core.JavaCore; import org.eclipse.jdt.launching.IVMInstall; import org.eclipse.jdt.launching.IVMInstallType; import org.eclipse.jdt.launching.JavaRuntime; @@ -96,12 +101,33 @@ public static String getVMInstallName(IExecutionEnvironment ee) throws CoreExcep IPath containerPath = JavaRuntime.newJREContainerPath(ee); IVMInstall vmi = JavaRuntime.getVMInstall(containerPath); if (vmi == null) { - throw new CoreException(createErrorStatus(NLS.bind(UtilMessages.VMHelper_noJreForExecEnv, ee.getId()))); + throw new CoreException(Status.error(NLS.bind(UtilMessages.VMHelper_noJreForExecEnv, ee.getId()))); } return vmi.getName(); } - public static IStatus createErrorStatus(String message) { - return Status.error(message, null); + private static final Map JAVA_VERSION_OF_EE = Arrays.stream(getExecutionEnvironments()) + .collect(Collectors.toMap(IExecutionEnvironment::getId, VMUtil::getJavaTargetVersion)); + + public static final Comparator ASCENDING_EE_JAVA_VERSION = Comparator + .comparingDouble((String ee) -> JAVA_VERSION_OF_EE.getOrDefault(ee, 0.0)) + .thenComparing(Comparator.naturalOrder()); + + public static double getJavaTargetVersion(IExecutionEnvironment ee) { + return switch (ee.getId()) { + // The EE's handled explicitly have unexpected java targets. + // See https://github.com/eclipse-equinox/equinox/pull/655 + case "J2SE-1.2" -> 1.2; //$NON-NLS-1$ + case "J2SE-1.3" -> 1.3; //$NON-NLS-1$ + case "J2SE-1.4" -> 1.4; //$NON-NLS-1$ + default -> { + Properties properties = ee.getProfileProperties(); + Object target = properties != null // + ? properties.get(JavaCore.COMPILER_CODEGEN_TARGET_PLATFORM) + : null; + yield target instanceof String version ? Double.parseDouble(version) : 0.0; + } + }; } + } diff --git a/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/BasePackageHeader.java b/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/BasePackageHeader.java index 162f756ac2..8f5e7728e8 100644 --- a/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/BasePackageHeader.java +++ b/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/BasePackageHeader.java @@ -13,6 +13,8 @@ *******************************************************************************/ package org.eclipse.pde.internal.core.text.bundle; +import java.util.Comparator; + import org.eclipse.osgi.util.ManifestElement; import org.eclipse.pde.internal.core.ICoreConstants; import org.eclipse.pde.internal.core.bundle.BundlePluginBase; @@ -25,7 +27,7 @@ public abstract class BasePackageHeader extends CompositeManifestHeader { private static final long serialVersionUID = 1L; public BasePackageHeader(String name, String value, IBundle bundle, String lineDelimiter) { - super(name, value, bundle, lineDelimiter, true); + super(name, value, bundle, lineDelimiter, Comparator.naturalOrder()); } protected String getVersionAttribute() { diff --git a/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/CompositeManifestHeader.java b/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/CompositeManifestHeader.java index c1b82bd0a2..e1c1e469ea 100644 --- a/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/CompositeManifestHeader.java +++ b/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/CompositeManifestHeader.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -33,21 +34,24 @@ public class CompositeManifestHeader extends ManifestHeader { private static final long serialVersionUID = 1L; private final boolean fSort; + private final Comparator fElementComparator; protected List fManifestElements; protected Map fElementMap; public CompositeManifestHeader(String name, String value, IBundle bundle, String lineDelimiter) { - this(name, value, bundle, lineDelimiter, false); + this(name, value, bundle, lineDelimiter, null); } - public CompositeManifestHeader(String name, String value, IBundle bundle, String lineDelimiter, boolean sort) { + public CompositeManifestHeader(String name, String value, IBundle bundle, String lineDelimiter, + Comparator elementComparator) { fName = name; fBundle = bundle; fLineDelimiter = lineDelimiter; setModel(fBundle.getModel()); - fSort = sort; + fSort = elementComparator != null; + fElementComparator = elementComparator; fValue = value; processValue(value); } @@ -120,7 +124,7 @@ protected void addManifestElement(PDEManifestElement element, boolean update) { element.setHeader(this); if (fSort) { if (fElementMap == null) { - fElementMap = new TreeMap<>(); + fElementMap = new TreeMap<>(fElementComparator); } fElementMap.put(element.getValue(), element); } else { diff --git a/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/RequiredExecutionEnvironmentHeader.java b/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/RequiredExecutionEnvironmentHeader.java index 532794add2..8bdc0f70ef 100644 --- a/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/RequiredExecutionEnvironmentHeader.java +++ b/ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/bundle/RequiredExecutionEnvironmentHeader.java @@ -19,13 +19,14 @@ import org.eclipse.osgi.util.ManifestElement; import org.eclipse.pde.internal.core.ibundle.IBundle; +import org.eclipse.pde.internal.core.util.VMUtil; public class RequiredExecutionEnvironmentHeader extends CompositeManifestHeader { private static final long serialVersionUID = 1L; public RequiredExecutionEnvironmentHeader(String name, String value, IBundle bundle, String lineDelimiter) { - super(name, value, bundle, lineDelimiter); + super(name, value, bundle, lineDelimiter, VMUtil.ASCENDING_EE_JAVA_VERSION); } @Override diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/model/bundle/ExecutionEnvironmentTestCase.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/model/bundle/ExecutionEnvironmentTestCase.java index 060ae443a8..6d75280bd5 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/model/bundle/ExecutionEnvironmentTestCase.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/model/bundle/ExecutionEnvironmentTestCase.java @@ -17,7 +17,6 @@ import static org.junit.Assert.assertNotNull; import java.util.HashSet; -import java.util.List; import java.util.Set; import org.eclipse.pde.internal.core.ibundle.IManifestHeader; @@ -131,10 +130,10 @@ public void testAddMulitplieExecutionEnvironmnets() throws Exception { int pos = fDocument.getLineOffset(3); int length = fDocument.getLineOffset(7) - fDocument.getLineOffset(3); assertEquals(""" - Bundle-RequiredExecutionEnvironment: J2SE-1.4, - CDC-1.1/Foundation-1.1, - J2SE-1.5, - OSGi/Minimum-1.1 + Bundle-RequiredExecutionEnvironment: CDC-1.1/Foundation-1.1, + OSGi/Minimum-1.1, + J2SE-1.4, + J2SE-1.5 """, fDocument.get(pos, length)); } @@ -150,8 +149,7 @@ public void testRemoveExecutionEnvironment() throws Exception { """); load(true); RequiredExecutionEnvironmentHeader header = getRequiredExecutionEnvironmentHeader(); - String env = header.getEnvironments().get(1); - header.removeExecutionEnvironment(env); + header.removeExecutionEnvironment("CDC-1.1/Foundation-1.1"); TextEdit[] ops = fListener.getTextOperations(); assertEquals(1, ops.length); @@ -163,8 +161,8 @@ public void testRemoveExecutionEnvironment() throws Exception { int pos = fDocument.getLineOffset(3); int length = fDocument.getLineOffset(5) - fDocument.getLineOffset(3); assertEquals(""" - Bundle-RequiredExecutionEnvironment: J2SE-1.4, - OSGi/Minimum-1.1 + Bundle-RequiredExecutionEnvironment: OSGi/Minimum-1.1, + J2SE-1.4 """, fDocument.get(pos, length)); } @@ -180,9 +178,8 @@ public void testRemoveMultipleExecutionEnvironments() throws Exception { """); load(true); RequiredExecutionEnvironmentHeader header = getRequiredExecutionEnvironmentHeader(); - List envs = header.getEnvironments(); - header.removeExecutionEnvironment(envs.get(1)); - header.removeExecutionEnvironment(envs.get(0)); + header.removeExecutionEnvironment("CDC-1.1/Foundation-1.1"); + header.removeExecutionEnvironment("J2SE-1.4"); TextEdit[] ops = fListener.getTextOperations(); assertEquals(1, ops.length); @@ -221,8 +218,8 @@ public void testPreserveSpacing() throws Exception { assertEquals(""" Bundle-RequiredExecutionEnvironment:\s - J2SE-1.4, - OSGi/Minimum-1.1 + OSGi/Minimum-1.1, + J2SE-1.4 """, fDocument.get(pos, length)); } diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java index 6d5e2c08bc..6062ddabd8 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java @@ -2905,10 +2905,6 @@ public class PDEUIMessages extends NLS { public static String RequiredExecutionEnvironmentSection_remove; - public static String RequiredExecutionEnvironmentSection_up; - - public static String RequiredExecutionEnvironmentSection_down; - public static String RequiredExecutionEnvironmentSection_fragmentDesc; public static String RequiredExecutionEnvironmentSection_pluginDesc; diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ExecutionEnvironmentSection.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ExecutionEnvironmentSection.java index 89b136379d..ca7d6be3ee 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ExecutionEnvironmentSection.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ExecutionEnvironmentSection.java @@ -112,7 +112,9 @@ public void dispose() { } public ExecutionEnvironmentSection(PDEFormPage page, Composite parent) { - super(page, parent, Section.DESCRIPTION, new String[] {PDEUIMessages.RequiredExecutionEnvironmentSection_add, PDEUIMessages.RequiredExecutionEnvironmentSection_remove, PDEUIMessages.RequiredExecutionEnvironmentSection_up, PDEUIMessages.RequiredExecutionEnvironmentSection_down}); + super(page, parent, Section.DESCRIPTION, new String[] { // + PDEUIMessages.RequiredExecutionEnvironmentSection_add, + PDEUIMessages.RequiredExecutionEnvironmentSection_remove }); createClient(getSection(), page.getEditor().getToolkit()); } @@ -210,8 +212,6 @@ protected void buttonSelected(int index) { switch (index) { case 0 -> handleAdd(); case 1 -> handleRemove(); - case 2 -> handleUp(); - case 3 -> handleDown(); } } @@ -245,30 +245,9 @@ public void run() { private void updateButtons() { Table table = fEETable.getTable(); - int count = table.getItemCount(); - boolean canMoveUp = count > 0 && table.getSelection().length == 1 && table.getSelectionIndex() > 0; - boolean canMoveDown = count > 0 && table.getSelection().length == 1 && table.getSelectionIndex() < count - 1; - TablePart tablePart = getTablePart(); tablePart.setButtonEnabled(0, isEditable()); tablePart.setButtonEnabled(1, isEditable() && table.getSelection().length > 0); - tablePart.setButtonEnabled(2, isEditable() && canMoveUp); - tablePart.setButtonEnabled(3, isEditable() && canMoveDown); - } - - private void handleDown() { - int selection = fEETable.getTable().getSelectionIndex(); - swap(selection, selection + 1); - } - - private void handleUp() { - int selection = fEETable.getTable().getSelectionIndex(); - swap(selection, selection - 1); - } - - public void swap(int index1, int index2) { - RequiredExecutionEnvironmentHeader header = getHeader(); - header.swap(index1, index2); } private void handleRemove() { diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties index 99e1f58f30..b35ee736a6 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties @@ -2283,8 +2283,6 @@ GetNonExternalizedStringsAction_allExternalizedMessage=All strings in manifest f RequiredExecutionEnvironmentSection_title=Execution Environments RequiredExecutionEnvironmentSection_add=Add... RequiredExecutionEnvironmentSection_remove=Remove -RequiredExecutionEnvironmentSection_up=Up -RequiredExecutionEnvironmentSection_down=Down RequiredExecutionEnvironmentSection_fragmentDesc=Specify the minimum execution environments required to run this fragment. RequiredExecutionEnvironmentSection_pluginDesc=Specify the minimum execution environments required to run this plug-in. RequiredExecutionEnvironmentSection_dialog_title=Execution Environments