Skip to content

Added test for E4WorkbenchModelEditor XMI tab search#2320

Open
r-mennig wants to merge 1 commit intoeclipse-pde:masterfrom
r-mennig:xmitab_search_test
Open

Added test for E4WorkbenchModelEditor XMI tab search#2320
r-mennig wants to merge 1 commit intoeclipse-pde:masterfrom
r-mennig:xmitab_search_test

Conversation

@r-mennig
Copy link
Copy Markdown
Contributor

Follow-up PR for #2293 adding a new test fragment for org.eclipse.e4.tools.emf.ui in order to test the search functionality of the XMI tab of the E4WorkbenchModelEditor.

The added regression test case asserts that the search highlighting reacts correctly to new key inputs into the search field and considers the full text of the search field for highlighting. The test data used for this is a simple project with a default application.

Comment on lines +102 to +130
private static void waitForUI() {
while (Display.getDefault().readAndDispatch()) {
// wait for UI
}
}

private static Event keyEvent(int key, int type, Widget widget) {
Event e = new Event();
e.keyCode = key;
e.character = (char) key;
e.type = type;
e.widget = widget;
return e;
}

private static IProject importTestProject(String path) throws IOException, CoreException {
URL entry = FileLocator
.toFileURL(FileLocator.find(FrameworkUtil.getBundle(XmiTabTest.class), IPath.fromOSString(path)));
if (entry == null) {
throw new IllegalArgumentException(path + " does not exist");
}
IPath projectFile = IPath.fromPortableString(entry.getPath()).append(IProjectDescription.DESCRIPTION_FILE_NAME);
IWorkspace workspace = ResourcesPlugin.getWorkspace();
IProjectDescription projectDescription = workspace.loadProjectDescription(projectFile);
IProject project = workspace.getRoot().getProject(projectDescription.getName());
project.create(projectDescription, null);
project.open(null);
return project;
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These helper methods could be replaced by existing utility methods (e.g. org.eclipse.pde.ui.tests.util.ProjectUtils), but I wasn't sure if depending on org.eclipse.pde.ui.tests would introduce some problems here.

@github-actions
Copy link
Copy Markdown

Test Results

  126 files  ±0    126 suites  ±0   39m 11s ⏱️ + 1m 51s
3 502 tests ±0  3 448 ✅ ±0   54 💤 ±0  0 ❌ ±0 
9 321 runs  ±0  9 191 ✅ ±0  130 💤 ±0  0 ❌ ±0 

Results for commit c80b3af. ± Comparison against base commit 5e3ba32.

@merks
Copy link
Copy Markdown
Contributor

merks commented Apr 29, 2026

@laeubi @HannesWell

Given there are no tests for the model editor, I assume it does make sense to add a new test bundle for that. Or would it be better to add it to this bundle?

image

@HannesWell
Copy link
Copy Markdown
Member

Sorry for joining the topic so late, but in general I wonder if the special search bar could be replaced with the general search overlay we have for editors since about a year?
Integrating that into the E4 editor would hopefully save code and would unify the UX.
Or is the search in the E4 editor more specialized/can do more?

If it should stay, I think it's fine to put the tests into the existing e4.tools.tests.

@r-mennig
Copy link
Copy Markdown
Contributor Author

r-mennig commented May 7, 2026

@HannesWell @merks I've tried looking into this.

Replacing the special search bar of the XMI tab with the generic find/replace logic should be fine as I could not find any differing behavior between the current search/highlighting logic and the generic one.

I've started the rework of the tab in this PR: r-mennig#1

What I couldn't get working however is having the search being shown as an overlay (instead of a dialog) since this is currently only implemented for instances of org.eclipse.ui.texteditor.StatusTextEditor (see org.eclipse.ui.texteditor.FindReplaceAction.shouldUseOverlay)

image

Maybe this check could be replaced by a less restrictive interface which would be implementable by the XMI tab?

@HannesWell
Copy link
Copy Markdown
Member

Maybe this check could be replaced by a less restrictive interface which would be implementable by the XMI tab?

From my POV there is no restriction on that, but I'm also not very much familiar with that specific part of the code.
@HeikoKlare was more involved in the implementation of the overlay sear, can you advice?

@HeikoKlare
Copy link
Copy Markdown
Contributor

If I remember correctly, with started with this kind of restriction because we had to properly integrate into the actual editor, which we initially only implemented for the StatusTextEditor. Most of the integration does actually not depend on a StatusTextEditor but on anything that can be adapted to an ITextViewer.

The crucial point, however, seems to be the disablement/enablement of actions for the workbench part into which the find/replace overlay is embedded. The workbench takes care of enabling the appropriate actions for the currently active part, but since the find/replace overlay is not a distinct part, the actions of the underlying editor (including copy/paste, undo/redo and the like) remain active. We have a very hacky implementation (using reflection, copied from JDT's breadcrumb implementation) that deactivates the editor's actions as long as the find/replace overlay has focus to work around this. One would have to extent this to all kinds of editors.
To me it seems as if this is not a proper design yet: probably the find/replace overlay needs to act as a separate workbench part that is realized as an overlay of and liked to the according editor. Then workbench functionality for ensuring proper action enablement should apply for the find/replace overlay without any manual hacks as well.
Maybe something like this goes into the right direction: eclipse-platform/eclipse.platform.ui#2093

We will try to follow up on this, as a proper solution could enable us to embed the find/replace functionality into many more kinds of views/editors.

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.

4 participants