Adapt PluginListPage to changes in PDE

[ptziegler]

The list of Plug-ins is now realized via a TableViewer, rather than a TreeViewer.

[ptziegler]

eclipse-pde/eclipse.pde#1666

[github-actions]

Test Results

0 files   -   324  0 suites   - 324   0s ⏱️ - 49m 48s
0 tests  -   686  0 ✅  -   664  0 💤  - 20  0 ❌  - 1 
0 runs   - 2 058  0 ✅  - 1 996  0 💤  - 60  0 ❌  - 1 

Results for commit ecd978f. ± Comparison against base commit 4cccdd4.

[vogella]

Would be nicer if M2E would not use internal API to avoid such situations

[laeubi]

Would be nicer if M2E would not use internal API to avoid such situations

Well this is the m2e PDE integration and of course it would be nice if PDE would have such things properly exposed in a way it can be safely reused instead of reimplemented... I just doubt anyone would like to take the effort for this.

@ptziegler we can't use "new" (internal) API unless a new release is performed. One option would be to use Viewer wherever possible and on the few places where we really need the concrete subclass to have an instanceof check so we can support both the Tree and the Table ...

[laeubi]

Suggested change

	class PluginContentProvider implements IStructuredContentProvider {
	class PluginContentProvider implements IStructuredContentProvider, ITreeContentProvider {

[ptziegler]

we can't use "new" (internal) API unless a new release is performed. One option would be to use Viewer wherever possible and on the few places where we really need the concrete subclass to have an instanceof check so we can support both the Tree and the Table ...

That won't be sufficient. If m2e won't use the nightly build of the PDE plugin then I can't explicitly reference the CachedCheckboxTableViewer class and therefore also can't call methods such as getAllCheckedElements(). Otherwise older IDEs that install the next M2E release will likely experience a NoClassDefFoundError or something similar when this class is loaded.

Unless I fall back to reflection, of course. But that doesn't just sound incredibly nasty, but will also look incredibly nasty.

[ptziegler]

That won't be sufficient. If m2e won't use the nightly build of the PDE plugin then I can't explicitly reference the CachedCheckboxTableViewer class and therefore also can't call methods such as getAllCheckedElements().

Unless of course the CachedCheckboxTableViewer is commited first as part of the 2025-06 release, but the changes to the BasePluginListPage only during the 2025-09 release...

[laeubi]

But that class extends CheckboxTableViewer cant we call e.g. org.eclipse.jface.viewers.CheckboxTableViewer.getCheckedElements() and that method is overridden instead of a new getAllCheckedElements() method (I'm not sure whats the difference between checked and all checked elements ..)

[ptziegler]

getCheckedElements() returns the checked elements after the filter has been applied. getAllCheckedElements() returns the unfiltered, checked elements. Perhaps it would be clearer if I rename the method to getUnfilteredCheckedElements()...

Even if I were to override this method now, it would still cause bugs with older versions of PDE, as those methods don't behave the same way once a filter is applied.

[laeubi]

Maybe m2e can simply register a CheckListener then?! In any case if it does not work and there is a M1 repo we can probably use that, and then m2e requires latest Eclipse release that's always been the case in the past, its just a bit unfortunate that it is the latest unreleased as usually we are only require RELEASE - 1 until there is RELEASE.

[ptziegler]

Maybe m2e can simply register a CheckListener then?! In any case if it does not work and there is a M1 repo we can probably use that, and then m2e requires latest Eclipse release that's always been the case in the past, its just a bit unfortunate that it is the latest unreleased as usually we are only require RELEASE - 1 until there is RELEASE.

Hm... I wonder about that. From what I can tell, the main difference between this PluginListPage and the PDE PluginListPage is that the handling of the launch configuration has been removed and instead, an additional handling for the MavenTargetLocation has been added.

Perhaps it would be more efficient to subclass the PluginListPage directly, disable the launch configuration and then there wouldn't be any need to handle the viewers at all.

[laeubi]

We can adapt to whatever seems sufficient here.

[ptziegler]

Someone else is free to continue if the changes to the CheckboxTablePart are merged to PDE. But at least from my perception, this becomes more trouble than it's worth.