Skip to content

UI support for setting filter attribute in feature editor #1942

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+129 −0
Open

UI support for setting filter attribute in feature editor #1942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/core/plugin/IPluginImport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ public interface IPluginImport extends IPluginObject, IPluginReference {
*/
String P_OPTIONAL = "optional"; //$NON-NLS-1$

/**
* @since 3.21
*/
String P_FILTER = "filter"; //$NON-NLS-1$

/**
* Tests whether the imported plug-in is reexported for
* plug-ins that will use this plug-in.
Expand Down Expand Up @@ -70,4 +75,17 @@ public interface IPluginImport extends IPluginObject, IPluginReference {
*/
void setOptional(boolean value) throws CoreException;

/**
* @since 3.21
*/
void setFilter(String filter) throws CoreException;

/**
* Returns the filter string for this plugin reference.
*
* @return the filter string, or null if not set
* @since 3.21
*/
String getFilter();

}
17 changes: 17 additions & 0 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginImport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public class PluginImport extends IdentifiablePluginObject implements IPluginImp
private boolean reexported = false;
private boolean optional = false;
private String version;
private String filter;

public PluginImport() {
}
Expand Down Expand Up @@ -74,6 +75,11 @@ public String getVersion() {
return version;
}

@Override
public String getFilter() {
return filter;
}

@Override
public boolean isReexported() {
return reexported;
Expand Down Expand Up @@ -203,6 +209,15 @@ public void setVersion(String version) throws CoreException {
firePropertyChanged(P_VERSION, oldValue, version);
}

@Override
public void setFilter(String filter) throws CoreException {
ensureModelEditable();
String oldValue = this.filter;
this.filter = filter;
firePropertyChanged(P_FILTER, oldValue, filter);

}

@Override
public void restoreProperty(String name, Object oldValue, Object newValue) throws CoreException {
if (name.equals(P_MATCH)) {
Expand Down Expand Up @@ -312,4 +327,6 @@ public void writeDelimeter(PrintWriter writer) {
writer.print(' ');
}



}
Comment on lines +330 to 332
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Extra blank lines at the end of the file should be removed to maintain consistent code formatting.

Copilot uses AI. Check for mistakes.
11 changes: 11 additions & 0 deletions ui/org.eclipse.pde.core/text/org/eclipse/pde/internal/core/text/plugin/PluginImportNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ public class PluginImportNode extends PluginObjectNode implements IPluginImport

private static final long serialVersionUID = 1L;

@Override
public void setFilter(String filter) {
setXMLAttribute(P_FILTER, filter);

}

@Override
public String getFilter() {
return getXMLAttributeValue(P_FILTER);
}

public PluginImportNode(String id) {
super();
String name = "plugin"; //$NON-NLS-1$
Expand Down
2 changes: 2 additions & 0 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/PDEUIMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ public class PDEUIMessages extends NLS {
public static String ContainerRenameParticipant_renameBundleId;

public static String ControlValidationUtility_errorMsgFilterInvalidSyntax;
public static String ControlValidationUtility_errorMsgFeatureFilterInvalidSyntax;
public static String ControlValidationUtility_errorMsgKeyNotFound;
public static String ControlValidationUtility_errorMsgNotOnClasspath;
public static String ControlValidationUtility_errorMsgPluginUnresolved;
Expand Down Expand Up @@ -977,6 +978,7 @@ public class PDEUIMessages extends NLS {
public static String ManifestEditor_MatchSection_equivalent;
public static String ManifestEditor_MatchSection_compatible;
public static String ManifestEditor_MatchSection_greater;
public static String ManifestEditor_MatchSection_filter;

public static String ManifestEditor_PluginSpecSection_title;
public static String ManifestEditor_PluginSpecSection_desc;
Expand Down
66 changes: 66 additions & 0 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/MatchSection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@
import org.eclipse.pde.core.plugin.IMatchRules;
import org.eclipse.pde.core.plugin.IPluginImport;
import org.eclipse.pde.core.plugin.IPluginReference;
import org.eclipse.pde.internal.core.ifeature.IFeatureImport;
import org.eclipse.pde.internal.core.plugin.ImportObject;
import org.eclipse.pde.internal.ui.PDEPlugin;
import org.eclipse.pde.internal.ui.PDEUIMessages;
import org.eclipse.pde.internal.ui.editor.FormEntryAdapter;
import org.eclipse.pde.internal.ui.editor.FormLayoutFactory;
import org.eclipse.pde.internal.ui.editor.PDEFormPage;
import org.eclipse.pde.internal.ui.editor.PDESection;
import org.eclipse.pde.internal.ui.editor.validation.ControlValidationUtility;
import org.eclipse.pde.internal.ui.editor.validation.TextValidator;
import org.eclipse.pde.internal.ui.parts.ComboPart;
import org.eclipse.pde.internal.ui.parts.FormEntry;
import org.eclipse.swt.SWT;
Expand All @@ -44,13 +47,18 @@
import org.eclipse.ui.forms.IPartSelectionListener;
import org.eclipse.ui.forms.widgets.FormToolkit;
import org.eclipse.ui.forms.widgets.Section;
import org.osgi.framework.FrameworkUtil;
import org.osgi.framework.InvalidSyntaxException;

public class MatchSection extends PDESection implements IPartSelectionListener {


Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Extra blank line after the class declaration should be removed for consistent formatting.

Suggested change

Copilot uses AI. Check for mistakes.
private Button fReexportButton;
private Button fOptionalButton;

private FormEntry fVersionText;
private FormEntry fFilterText;
private TextValidator fFilterEntryValidator;

private ComboPart fMatchCombo;
protected IPluginReference fCurrentImport;
Expand Down Expand Up @@ -134,6 +142,32 @@ public void textDirty(FormEntry text) {
}));
toolkit.paintBordersFor(container);
initialize();

fFilterText = new FormEntry(container, toolkit, PDEUIMessages.ManifestEditor_MatchSection_filter, null, false);
fFilterText.setFormEntryListener(
new FormEntryAdapter(this, getPage().getEditor().getEditorSite().getActionBars()) {
@Override
public void textValueChanged(FormEntry text) {
applyFilter(text.getValue());
}

@Override
public void textDirty(FormEntry text) {
if (fBlockChanges) {
return;
}
markDirty();
fBlockChanges = true;
resetMatchCombo(fCurrentImport);
fBlockChanges = false;
}
});
fFilterEntryValidator = new TextValidator(getManagedForm(), fFilterText.getText(), getProject(), true) {
@Override
protected boolean validateControl() {
return validateFilter();
}
};
update((IPluginReference) null);

section.setClient(container);
Expand All @@ -143,6 +177,16 @@ public void textDirty(FormEntry text) {
section.setLayoutData(new GridData(GridData.FILL_HORIZONTAL | GridData.VERTICAL_ALIGN_BEGINNING));
}

private boolean validateFilter() {
// No validation required for an optional field
if (fFilterText.getText().getText().length() == 0) {
return true;
}
// Value must match the current environment and contain valid syntax
return ControlValidationUtility.validateFilterField(fFilterText.getText().getText(),
fFilterEntryValidator);
}

private void createReexportButton(FormToolkit toolkit, Composite container) {
fReexportButton = toolkit.createButton(container, PDEUIMessages.ManifestEditor_MatchSection_reexport, SWT.CHECK);
fReexportButton.addSelectionListener(widgetSelectedAdapter(e -> {
Expand Down Expand Up @@ -200,6 +244,22 @@ private void applyMatch(int match) {
}
}

private void applyFilter(String filter) {
try {
if (fCurrentImport == null)
return;
if (filter == null || filter.isEmpty())
((IFeatureImport) fCurrentImport).setFilter(null);
else {
FrameworkUtil.createFilter(filter);
((IFeatureImport) fCurrentImport).setFilter(filter);
}
} catch (InvalidSyntaxException e) {
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The InvalidSyntaxException is silently swallowed without logging or user feedback. While validation occurs in validateFilter(), the exception here indicates the filter was accepted by the validator but failed when creating the actual filter. This scenario should be logged for debugging purposes, similar to how CoreException is handled.

Suggested change
} catch (InvalidSyntaxException e) {
} catch (InvalidSyntaxException e) {
PDEPlugin.logException(e);

Copilot uses AI. Check for mistakes.
} catch (CoreException e) {
PDEPlugin.logException(e);
}
}
Comment on lines +247 to +261
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The method unconditionally casts fCurrentImport to IFeatureImport, but MatchSection is also used for plugin imports (IPluginImport) in DependenciesPage. This will cause a ClassCastException when used in the plugin context. The code should check the type and cast appropriately to either IPluginImport or IFeatureImport, both of which support the filter property.

Copilot uses AI. Check for mistakes.

private int getMatch() {
return fMatchCombo.getSelectionIndex();
}
Expand Down Expand Up @@ -272,8 +332,11 @@ protected void update(IPluginReference iimport) {
}
fVersionText.setValue(null, true);
fVersionText.setEditable(false);

fMatchCombo.getControl().setEnabled(false);
fMatchCombo.setText(""); //$NON-NLS-1$
fFilterText.setValue(null, false);
fFilterText.setEditable(false);
fCurrentImport = null;
fBlockChanges = false;
return;
Expand All @@ -292,7 +355,10 @@ protected void update(IPluginReference iimport) {
}
fVersionText.setEditable(isEditable());
fVersionText.setValue(fCurrentImport.getVersion());

resetMatchCombo(fCurrentImport);
fFilterText.setEditable(isEditable());
fFilterText.setValue(((IFeatureImport) fCurrentImport).getFilter());
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The unconditional cast to IFeatureImport will cause a ClassCastException when MatchSection is used in the plugin dependencies context where fCurrentImport is an IPluginImport. Check the instance type and cast appropriately, as both IPluginImport and IFeatureImport support the filter property.

Suggested change
fFilterText.setValue(((IFeatureImport) fCurrentImport).getFilter());
fFilterText.setValue(fCurrentImport.getFilter());

Copilot uses AI. Check for mistakes.
fBlockChanges = false;
}
}
13 changes: 13 additions & 0 deletions ...se.pde.ui/src/org/eclipse/pde/internal/ui/editor/validation/ControlValidationUtility.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,19 @@ public static boolean validatePlatformFilterField(String value, IValidatorMessag
return true;
}

public static boolean validateFilterField(String value, IValidatorMessageHandler validator) {
// Check to see if the platform filter syntax is valid
try {
PDECore.getDefault().getBundleContext().createFilter(value);
} catch (InvalidSyntaxException ise) {
validator.addMessage(PDEUIMessages.ControlValidationUtility_errorMsgFeatureFilterInvalidSyntax,
IMessageProvider.ERROR);
return false;
}

return true;
}

public static boolean validateActivatorField(String value, IValidatorMessageHandler validator, IProject project) {

// Check the compiler flag and translate it into a message type
Expand Down
2 changes: 2 additions & 0 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/pderesources.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ ManifestEditor_MatchSection_perfect = Perfect
ManifestEditor_MatchSection_equivalent = Equivalent
ManifestEditor_MatchSection_compatible = Compatible
ManifestEditor_MatchSection_greater = Greater or Equal
ManifestEditor_MatchSection_filter = Filter:

ManifestEditor_PluginSpecSection_title = General Information
ManifestEditor_PluginSpecSection_desc = This section describes general information about this plug-in.
Expand Down Expand Up @@ -2523,6 +2524,7 @@ ControlValidationUtility_errorMsgValueMustBeSpecified=A value must be specified
ControlValidationUtility_errorMsgValueNotExternalized=The specified value is not externalized
ControlValidationUtility_errorMsgKeyNotFound=The specified key is not present in the plug-in's properties file
ControlValidationUtility_errorMsgFilterInvalidSyntax=The specified platform filter contains invalid syntax
ControlValidationUtility_errorMsgFeatureFilterInvalidSyntax=The specified feature filter contains invalid syntax
PackageFinder_taskName=Searching class files for package references

UpdateSingleton_dir_label=Update ''{0}'' to 'singleton' directive true
Expand Down
Loading