Skip to content

Issue 2005 import spy adapter#2151

Open
amelodev wants to merge 4 commits into
eclipse-pde:masterfrom
amelodev:issue-2005_ImportSpyAdapter
Open

Issue 2005 import spy adapter#2151
amelodev wants to merge 4 commits into
eclipse-pde:masterfrom
amelodev:issue-2005_ImportSpyAdapter

Conversation

@amelodev
Copy link
Copy Markdown
Contributor

@amelodev amelodev commented Dec 4, 2025

Fixes #2005

org.eclipse.pde.spy.adapter bundle added to the PDE spies collection.
This new spy provides a view of the available adapter factories.

There is one issue I have not been able to resolve yet — the Adapter Spy shortcut does not seem to work.

This problem also appears to affect other spies such as the Event Spy and the Context Spy.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 4, 2025

Test Results

  126 files  ±0    126 suites  ±0   38m 16s ⏱️ + 5m 37s
3 502 tests  - 1  3 448 ✅ +69   54 💤 ±0  0 ❌  - 69 
9 321 runs   - 1  9 191 ✅ +74  130 💤 ±0  0 ❌  - 74 

Results for commit db2393c. ± Comparison against base commit 5c5792d.

This pull request removes 1 test.
TestPDETemplates ‑ [3: RCP 3.x application (minimal)]

♻️ This comment has been updated with latest results.

@laeubi laeubi requested a review from HannesWell January 17, 2026 07:08
@laeubi
Copy link
Copy Markdown
Contributor

laeubi commented Jan 17, 2026

We would need a CQ here to import the code into PDE if approved by PL.
Smaller issues can be fixed afterwards.

@amelodev
Copy link
Copy Markdown
Contributor Author

Copy link
Copy Markdown
Member

@iloveeclipse iloveeclipse left a comment

Choose a reason for hiding this comment

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

I did a short smoke test, no real code review but just some few basic checks.

In general looks useful, however I was missing some adapters and couldn't really figure out why.

For example, I've missed java.io.File adapter factory contributed here
https://github.com/iloveeclipse/anyedittools/blob/a8eaa4995f7724b871375d250e87dcdd2973c82b/AnyEditTools/plugin.xml#L626-L632

and here:

https://github.com/iloveeclipse/anyedittools/blob/a8eaa4995f7724b871375d250e87dcdd2973c82b/AnyEditTools/src/de/loskutov/anyedit/ui/editor/EditorAdapterFactory.java#L70-L72

Also three filtering seem to work in a very strange way, even if table shows java.io.File, entering File in the filter sometimes shows some unrelated matches but not java.io.File, and sometimes it shows it.

Beside this, please rebase (not merge) your branch on top of latest master commit.

Comment thread ui/org.eclipse.pde.spy.adapter/pom.xml Outdated
Comment thread ui/org.eclipse.pde.spy.adapter/about.html
Comment thread ui/org.eclipse.pde.spy.adapter/notice.html
Comment thread ui/org.eclipse.pde.spy.adapter/.classpath Outdated
Comment thread ui/org.eclipse.pde.spy.adapter/icons/ext_point_obj.png
Comment thread ui/org.eclipse.pde.spy.adapter/META-INF/MANIFEST.MF Outdated
Comment thread ui/org.eclipse.pde.spy.adapter/.settings/org.eclipse.jdt.core.prefs Outdated
@amelodev amelodev force-pushed the issue-2005_ImportSpyAdapter branch from 192d9ab to 5369f4e Compare February 20, 2026 13:56
@iloveeclipse
Copy link
Copy Markdown
Member

@amelodev : please could you mark all comments on PR as resolved if they are already addressed by the latest change, and those that are unresolved give a comment why not?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR imports the adapter spy plugin (org.eclipse.pde.spy.adapter) from an external source into the PDE spies collection. The adapter spy provides a view to display all available adapter factories in the Eclipse runtime, showing which source types can be adapted to which destination types. The plugin was originally developed by lacherp and has been renamed from org.eclipse.e4.tools.adapter.spy to fit the PDE naming convention.

Changes:

  • Added new org.eclipse.pde.spy.adapter bundle with complete source code, icons, and metadata
  • Includes model classes for adapter data representation, viewer components for tree display, and helper utilities for adapter introspection
  • Integrated with the PDE spy framework using the org.eclipse.pde.spy.core.spyPart extension point

Reviewed changes

Copilot reviewed 24 out of 30 changed files in this pull request and generated 22 comments.

Show a summary per file
File Description
META-INF/MANIFEST.MF Bundle manifest defining dependencies and execution environment (JavaSE-21)
plugin.xml Extension point registration for the adapter spy part
plugin.properties Localized bundle metadata and descriptions
build.properties Build configuration specifying source and output directories
.project/.classpath/.settings/* Eclipse project configuration files
src/org/eclipse/pde/spy/adapter/AdapterSpyPart.java Main UI part creating the tree viewer and toolbar controls
src/org/eclipse/pde/spy/adapter/Messages.java NLS message accessor class
src/org/eclipse/pde/spy/adapter/Messages.properties Internationalized message strings
src/org/eclipse/pde/spy/adapter/model/* Model classes (AdapterData, AdapterRepository, AdapterElementType)
src/org/eclipse/pde/spy/adapter/viewer/* Viewer support classes (content providers, filters, comparators)
src/org/eclipse/pde/spy/adapter/tools/AdapterHelper.java Helper utilities for adapter introspection
src/org/eclipse/pde/spy/adapter/hook/EclipseAdapterHook.java Hook to intercept Eclipse adapter calls
icons/* Image resources for the spy UI
about.html/notice.html Legal documentation files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ui/org.eclipse.pde.spy.adapter/META-INF/MANIFEST.MF
Comment thread ui/org.eclipse.pde.spy.adapter/src/org/eclipse/pde/spy/adapter/Messages.java Outdated
Comment thread ui/org.eclipse.pde.spy.adapter/build.properties Outdated
Comment thread ui/org.eclipse.pde.spy.adapter/build.properties
@amelodev amelodev force-pushed the issue-2005_ImportSpyAdapter branch from 7a316c6 to c39d5c7 Compare February 24, 2026 16:22
@amelodev amelodev force-pushed the issue-2005_ImportSpyAdapter branch from c39d5c7 to 1445add Compare March 17, 2026 10:36
@amelodev
Copy link
Copy Markdown
Contributor Author

amelodev commented May 5, 2026

What needs to be done to get this PR merged?

@merks
Copy link
Copy Markdown
Contributor

merks commented May 5, 2026

Things seem to have stalled

It should be rebased on master.

All copyrights should have a year:

Copyright (c)  Lacherp.

The *.png are a bit problematic and will need to be replaced by *.svg, but I assume we can do that after the fact.

The formatting looks poor in place:

if( l instanceof IAdapterFactoryExt) {

I think there should be more things in the .settings so that the save actions kick. Look in other projects at the contents of the .settings folder.

Some of these things should use pattern matching instanceof

image

I personally don't think a .foreach should be used for a simple for loop will do nicely.

I think quite some detail reviews are needed here but who is going to do that?

@iloveeclipse @laeubi @HannesWell

@iloveeclipse
Copy link
Copy Markdown
Member

What needs to be done to get this PR merged?

Beside free time & reviewers interested in this change:

  1. Answer to this comment and eventually fixes.
  2. Comprehensive review, there is a lot of code.
  3. Rebase on master, branch is a bit outdated, so we see if tests are running or not.
  4. Fix all review comments from step before.
  5. Wait for 4.41 integration (I guess this pR will not make into 4.40 anymore)

Since I personally have no time for a comprehensive review, I will trigger Copilot review once again, it seem to do "randomized" reviews where some parts of the code are reviewed in subsequent runs, and some not...

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 24 out of 30 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +74 to +78
AdapterData adapData = new AdapterData(AdapterElementType.DESTINATION_TYPE);
adapData.setParent(refAdapterData.get());
adapData.setDestinationType(targetType);
destinationTypeToAdapterDataMap.put(targetType, adapData);
refClassName.set("");
Comment on lines +100 to +105
if (((AdapterData)ad.getParent()).getSourceType().equals(adchild.getDestinationType())) {
AdapterData adpd=new AdapterData(adchild);
ad.getChildrenList().add(adpd);
return;
}
ad.getChildrenList().add(adchild);
this.getChildrenList().forEach(child -> {
AdapterData newAdapterData = new AdapterData(child);
AdapterData soon = new AdapterData(this);
soon.setParent(child);
Comment on lines +161 to +174
public Object getChildren(boolean sourceToDestination) {
if (!children.isEmpty()) {
Collections.sort(children);
if( sourceToDestination) {
Map<String, List<AdapterData>> childs = children.stream().collect(Collectors.groupingBy(AdapterData::getDestinationType));
children.clear();
childs.values().forEach(ls-> children.add(ls.get(0)));
return children.toArray();
}else {
Map<String, List<AdapterData>> childs = children.stream().collect(Collectors.groupingBy(AdapterData::getSourceType));
children.clear();
childs.values().forEach(ls-> children.add(ls.get(0)));
return children.toArray();
}
}
// reduce source Type
Collection<AdapterData> reduceresult = reduceType(adapaters);
refreshAdapterTree(NAMED_UPDATE_TREE_SOURCE_TO_DESTINATION, reduceresult);
Comment on lines +276 to +284
Clipboard clipboard = new Clipboard(null);
try {
TextTransfer textTransfer = TextTransfer.getInstance();
Transfer[] transfers = new Transfer[] { textTransfer };
Object[] data = new Object[] { toCopy };
clipboard.setContents(data, transfers);
} finally {
clipboard.dispose();
}
Comment on lines +13 to +15
org.eclipse.emf.ecore,
org.eclipse.jface,
org.eclipse.equinox.common,
@amelodev amelodev force-pushed the issue-2005_ImportSpyAdapter branch from 1445add to db2393c Compare May 6, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should import the nice adapter spy made by lacherp

5 participants