diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/AllTargetTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/AllTargetTests.java index e7ff597cec6..e5a5d08da71 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/AllTargetTests.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/AllTargetTests.java @@ -25,7 +25,8 @@ TargetDefinitionResolutionTests.class, // TargetDefinitionFeatureResolutionTests.class, // IUBundleContainerTests.class, // - ProfileContainerTests.class }) + ProfileContainerTests.class, // + StyledBundleLabelProviderTests.class }) public class AllTargetTests { } diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/StyledBundleLabelProviderTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/StyledBundleLabelProviderTests.java new file mode 100644 index 00000000000..5b5bca32a20 --- /dev/null +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/StyledBundleLabelProviderTests.java @@ -0,0 +1,190 @@ +/******************************************************************************* + * Copyright (c) 2026 Lars Vogel 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 + *******************************************************************************/ +package org.eclipse.pde.ui.tests.target; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.nio.file.Path; +import java.util.Arrays; + +import org.eclipse.pde.core.target.ITargetDefinition; +import org.eclipse.pde.core.target.ITargetLocation; +import org.eclipse.pde.core.target.TargetBundle; +import org.eclipse.pde.internal.ui.shared.target.StyledBundleLabelProvider; +import org.junit.Test; + +/** + * Tests for {@link StyledBundleLabelProvider}, in particular the optional + * inline source-location suffix that is appended to {@link TargetBundle} + * labels when a target context is set. + */ +public class StyledBundleLabelProviderTests extends AbstractTargetTest { + + private static final String SEPARATOR = " - "; //$NON-NLS-1$ + + /** + * Without a target context the label must not carry any source-location + * suffix. + */ + @Test + public void testBundleLabelWithoutTargetContextOmitsSource() throws Exception { + Path dirPath = extractAbcdePlugins().resolve("plugins"); + ITargetDefinition definition = getNewTarget(); + ITargetLocation container = getTargetService().newDirectoryLocation(dirPath.toString()); + definition.setTargetLocations(new ITargetLocation[] { container }); + definition.resolve(null); + + TargetBundle[] bundles = definition.getAllBundles(); + assertNotNull("Expected resolved bundles", bundles); + assertTrue("Expected at least one bundle", bundles.length > 0); + + StyledBundleLabelProvider provider = new StyledBundleLabelProvider(true, false); + try { + for (TargetBundle bundle : bundles) { + String text = provider.getText(bundle); + assertFalse("Label must not contain source suffix without target context: " + text, + text.contains(SEPARATOR + dirPath.toString())); + } + } finally { + provider.dispose(); + } + } + + /** + * With a target context set, every bundle label must be suffixed with its + * originating location so duplicates can be distinguished. + */ + @Test + public void testBundleLabelWithTargetContextAppendsSource() throws Exception { + Path dirPath = extractAbcdePlugins().resolve("plugins"); + ITargetDefinition definition = getNewTarget(); + ITargetLocation container = getTargetService().newDirectoryLocation(dirPath.toString()); + definition.setTargetLocations(new ITargetLocation[] { container }); + definition.resolve(null); + + TargetBundle[] bundles = definition.getAllBundles(); + assertNotNull("Expected resolved bundles", bundles); + assertTrue("Expected at least one bundle", bundles.length > 0); + + StyledBundleLabelProvider provider = new StyledBundleLabelProvider(true, false); + try { + provider.setTargetContext(definition); + for (TargetBundle bundle : bundles) { + String text = provider.getText(bundle); + assertTrue("Label should end with source location suffix but was: " + text, + text.endsWith(SEPARATOR + dirPath.toString())); + } + } finally { + provider.dispose(); + } + } + + /** + * When a bundle exists in two different locations, each resolved + * {@link TargetBundle} must carry its owning location's path so the user + * can tell duplicates apart — the scenario that motivated this feature. + */ + @Test + public void testBundleLabelDistinguishesDuplicatesFromDifferentLocations() throws Exception { + Path abcdePath = extractAbcdePlugins().resolve("plugins"); + Path multiPath = extractMultiVersionPlugins(); + + ITargetDefinition definition = getNewTarget(); + ITargetLocation abcdeLocation = getTargetService().newDirectoryLocation(abcdePath.toString()); + ITargetLocation multiLocation = getTargetService().newDirectoryLocation(multiPath.toString()); + definition.setTargetLocations(new ITargetLocation[] { abcdeLocation, multiLocation }); + definition.resolve(null); + + TargetBundle[] abcdeBundles = abcdeLocation.getBundles(); + TargetBundle[] multiBundles = multiLocation.getBundles(); + assertNotNull("Expected abcde bundles", abcdeBundles); + assertNotNull("Expected multi-version bundles", multiBundles); + assertTrue("Expected abcde location to contribute bundles", abcdeBundles.length > 0); + assertTrue("Expected multi-version location to contribute bundles", multiBundles.length > 0); + + StyledBundleLabelProvider provider = new StyledBundleLabelProvider(true, false); + try { + provider.setTargetContext(definition); + for (TargetBundle bundle : abcdeBundles) { + String text = provider.getText(bundle); + assertTrue("abcde bundle label should carry abcde path but was: " + text, + text.endsWith(SEPARATOR + abcdePath.toString())); + } + for (TargetBundle bundle : multiBundles) { + String text = provider.getText(bundle); + assertTrue("multi-version bundle label should carry multi path but was: " + text, + text.endsWith(SEPARATOR + multiPath.toString())); + } + } finally { + provider.dispose(); + } + } + + /** + * Clearing the target context restores the unsuffixed label, so the + * provider can be reused across editors. + */ + @Test + public void testClearingTargetContextRemovesSource() throws Exception { + Path dirPath = extractAbcdePlugins().resolve("plugins"); + ITargetDefinition definition = getNewTarget(); + ITargetLocation container = getTargetService().newDirectoryLocation(dirPath.toString()); + definition.setTargetLocations(new ITargetLocation[] { container }); + definition.resolve(null); + + TargetBundle bundle = definition.getAllBundles()[0]; + StyledBundleLabelProvider provider = new StyledBundleLabelProvider(true, false); + try { + provider.setTargetContext(definition); + String withContext = provider.getText(bundle); + assertTrue("Expected source suffix when context is set: " + withContext, + withContext.endsWith(SEPARATOR + dirPath.toString())); + + provider.setTargetContext(null); + String withoutContext = provider.getText(bundle); + assertFalse("Expected no source suffix after context cleared: " + withoutContext, + withoutContext.contains(SEPARATOR + dirPath.toString())); + } finally { + provider.dispose(); + } + } + + /** + * The styled string for a resolved bundle without target context should + * still start with the bundle's symbolic name — a sanity check that the + * new code does not disturb the base label. + */ + @Test + public void testBundleLabelStartsWithSymbolicName() throws Exception { + Path dirPath = extractAbcdePlugins().resolve("plugins"); + ITargetDefinition definition = getNewTarget(); + ITargetLocation container = getTargetService().newDirectoryLocation(dirPath.toString()); + definition.setTargetLocations(new ITargetLocation[] { container }); + definition.resolve(null); + + TargetBundle[] bundles = definition.getAllBundles(); + assertTrue("Expected resolved abcde bundles", bundles.length > 0); + + StyledBundleLabelProvider provider = new StyledBundleLabelProvider(true, false); + try { + TargetBundle a = Arrays.stream(bundles) + .filter(b -> "bundle.a".equals(b.getBundleInfo().getSymbolicName())).findFirst() + .orElse(bundles[0]); + String text = provider.getText(a); + assertEquals(a.getBundleInfo().getSymbolicName(), text.split(" ")[0]); + } finally { + provider.dispose(); + } + } +} diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/StyledBundleLabelProvider.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/StyledBundleLabelProvider.java index 101f889dfec..823374bf518 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/StyledBundleLabelProvider.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/StyledBundleLabelProvider.java @@ -32,6 +32,7 @@ import org.eclipse.jface.viewers.StyledCellLabelProvider; import org.eclipse.jface.viewers.StyledString; import org.eclipse.jface.viewers.ViewerCell; +import org.eclipse.pde.core.target.ITargetDefinition; import org.eclipse.pde.core.target.ITargetLocation; import org.eclipse.pde.core.target.NameVersionDescriptor; import org.eclipse.pde.core.target.TargetBundle; @@ -60,6 +61,7 @@ public class StyledBundleLabelProvider extends StyledCellLabelProvider implement private boolean fShowVersion = true; private boolean fAppendResolvedVariables = false; + private ITargetDefinition fTargetContext; @SuppressWarnings("restriction") private final org.eclipse.equinox.internal.p2.metadata.TranslationSupport fTranslations = org.eclipse.equinox.internal.p2.metadata.TranslationSupport .getInstance(); @@ -79,6 +81,20 @@ public StyledBundleLabelProvider(boolean showVersion, boolean appendResolvedVari fAppendResolvedVariables = appendResolvedVariables; } + /** + * Sets the target definition used to resolve the originating + * {@link ITargetLocation} for a {@link TargetBundle}. When set, bundle + * labels are suffixed with a short description of the source location so + * that duplicates from different sources can be distinguished. + * + * @param context + * target definition providing the locations to search, or + * null to disable the source suffix + */ + public void setTargetContext(ITargetDefinition context) { + fTargetContext = context; + } + @Override public void dispose() { PDEPlugin.getDefault().getLabelProvider().disconnect(this); @@ -134,10 +150,12 @@ private StyledString getInternalStyledString(Object element) { IStatus status = bundle.getStatus(); if (status.isOK()) { appendBundleInfo(styledString, bundle.getBundleInfo()); + appendSourceLocation(styledString, bundle); } else { BundleInfo bundleInfo = bundle.getBundleInfo(); if (bundleInfo != null && bundleInfo.getSymbolicName() != null) { appendBundleInfo(styledString, bundleInfo); + appendSourceLocation(styledString, bundle); styledString.append(' '); } styledString.append(status.getMessage()); @@ -259,6 +277,68 @@ private void appendLocation(StyledString styledString, ITargetLocation container } } + /** + * Appends a short description of the {@link ITargetLocation} the given + * bundle was resolved from, if a target context has been set via + * {@link #setTargetContext(ITargetDefinition)} and the bundle's originating + * location can be found. + */ + private void appendSourceLocation(StyledString styledString, TargetBundle bundle) { + ITargetLocation source = findSourceLocation(bundle); + if (source == null) { + return; + } + String label = getSourceLocationLabel(source); + if (label == null || label.isEmpty()) { + return; + } + styledString.append(" - ", StyledString.DECORATIONS_STYLER); //$NON-NLS-1$ + styledString.append(label, StyledString.DECORATIONS_STYLER); + } + + private ITargetLocation findSourceLocation(TargetBundle bundle) { + if (fTargetContext == null) { + return null; + } + ITargetLocation[] locations = fTargetContext.getTargetLocations(); + if (locations == null) { + return null; + } + for (ITargetLocation location : locations) { + TargetBundle[] bundles = location.getBundles(); + if (bundles == null) { + continue; + } + for (TargetBundle b : bundles) { + if (b == bundle) { + return location; + } + } + } + return null; + } + + private String getSourceLocationLabel(ITargetLocation container) { + try { + if (container instanceof FeatureBundleContainer feature) { + String id = feature.getFeatureId(); + String version = feature.getFeatureVersion(); + return version != null ? id + " [" + version + "]" : id; //$NON-NLS-1$ //$NON-NLS-2$ + } + if (container instanceof IUBundleContainer iu) { + List repos = iu.getRepositories(); + return repos.isEmpty() ? Messages.BundleContainerTable_8 : repos.get(0).toString(); + } + String location = container.getLocation(false); + if (location != null) { + return location; + } + } catch (CoreException e) { + // fall through to type + } + return container.getType(); + } + /** * Appends a label describing the number of bundles included (ex. 5 of 10 * plug-ins). diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/TargetContentsGroup.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/TargetContentsGroup.java index cc0fc0b3e0d..c78760f461b 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/TargetContentsGroup.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/TargetContentsGroup.java @@ -117,6 +117,7 @@ public class TargetContentsGroup { private ViewerFilter fSourceFilter; private ViewerFilter fPluginFilter; + private StyledBundleLabelProvider fLabelProvider; private TargetDefinition fTargetDefinition; /** @@ -266,7 +267,8 @@ private TreeViewer createTree(Composite parent, FormToolkit toolkit) { fTree.getControl().setFont(parent.getFont()); fTree.setUseHashlookup(true); fTree.setContentProvider(new TreeContentProvider()); - fTree.setLabelProvider(new StyledBundleLabelProvider(true, false)); + fLabelProvider = new StyledBundleLabelProvider(true, false); + fTree.setLabelProvider(fLabelProvider); fTree.addDoubleClickListener(event -> { IStructuredSelection selection = (IStructuredSelection) event.getSelection(); Object first = selection.getFirstElement(); @@ -912,6 +914,10 @@ public void setInput(ITargetDefinition input) { return; } + if (fLabelProvider != null) { + fLabelProvider.setTargetContext(fTargetDefinition); + } + if (!input.isResolved()) { fTree.setInput(Messages.TargetContentsGroup_10); setEnabled(false); @@ -974,6 +980,9 @@ private void updateCheckState() { */ public void setCancelled() { fTargetDefinition = null; + if (fLabelProvider != null) { + fLabelProvider.setTargetContext(null); + } fTree.setInput(Messages.TargetContentsGroup_resolveCancelled); setEnabled(false); }