diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java index 85883ec0ada..d4dc5f85cfc 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java @@ -704,14 +704,20 @@ private boolean openUnifiedDiff(UnifiedDiffSource source, CompareEditorInput inp // no active workbench page; fall back to the classic compare editor path return false; } + // Only an editor opened here may be closed again when the diff cannot be + // applied after all; one the user already had open stays untouched. + IEditorPart editorBefore = wpage.findEditor(source.editorInput()); + IEditorPart openedHere = null; try { IDocumentMergerInput mergerInput = findDocumentMergerInput(input, source.compareInput()); IEditorPart editorPart = wpage.openEditor(source.editorInput(), getEditorId(source.editorInput(), source.element())); + openedHere = editorPart == editorBefore ? null : editorPart; if (editorPart instanceof MultiPageEditorPart mpe && mpe.getSelectedPage() instanceof IEditorPart selected) { editorPart = selected; } if (!(editorPart instanceof ITextEditor textEditor)) { + closeIfOpenedHere(wpage, openedHere); return false; } Action openTwoWayCompare = createOpenTwoWayCompareAction(input, page, editor, activate, textEditor); @@ -726,13 +732,25 @@ private boolean openUnifiedDiff(UnifiedDiffSource source, CompareEditorInput inp .open(); // The user canceled the diff, not the open: leave the text editor alone // instead of falling back to the classic compare editor. - return status.isOK() || status == UnifiedDiffManager.CANCELED_BY_USER; + if (status.isOK() || status == UnifiedDiffManager.CANCELED_BY_USER) { + return true; + } } catch (PartInitException e) { CompareUIPlugin.log(e); } + // The classic compare editor takes over, so the editor opened for the unified + // diff would only be a second editor on the same file without a comparison. + closeIfOpenedHere(wpage, openedHere); return false; } + private static void closeIfOpenedHere(IWorkbenchPage page, IEditorPart editor) { + if (editor != null) { + // Nothing here writes to the document, so there is nothing to save. + page.closeEditor(editor, false); + } + } + /** * Returns the token comparator and whitespace factories of a registered custom * merge viewer, or null when the platform's own text merge viewer diff --git a/team/tests/org.eclipse.compare.tests/build.properties b/team/tests/org.eclipse.compare.tests/build.properties index 03eb29630d2..ffab92f3f3f 100644 --- a/team/tests/org.eclipse.compare.tests/build.properties +++ b/team/tests/org.eclipse.compare.tests/build.properties @@ -13,6 +13,7 @@ # Mickael Istria (Red Hat Inc.) - 419531 Get rid of nested jars ############################################################################### bin.includes = plugin.properties,\ + plugin.xml,\ test.xml,\ about.html,\ .,\ diff --git a/team/tests/org.eclipse.compare.tests/plugin.xml b/team/tests/org.eclipse.compare.tests/plugin.xml new file mode 100644 index 00000000000..3e8a8b88615 --- /dev/null +++ b/team/tests/org.eclipse.compare.tests/plugin.xml @@ -0,0 +1,24 @@ + + + + + + + + + diff --git a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/NonTextTestEditor.java b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/NonTextTestEditor.java new file mode 100644 index 00000000000..dcef8256fc3 --- /dev/null +++ b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/NonTextTestEditor.java @@ -0,0 +1,70 @@ +/******************************************************************************* + * 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 + * + * Contributors: + * Eclipse contributors - initial API and implementation + *******************************************************************************/ +package org.eclipse.compare.tests; + +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.swt.SWT; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Label; +import org.eclipse.ui.IEditorInput; +import org.eclipse.ui.IEditorSite; +import org.eclipse.ui.part.EditorPart; + +/** + * An editor that is deliberately not an {@link org.eclipse.ui.texteditor.ITextEditor}. + * Registered for the {@code nontexteditortest} extension so tests can open a file + * whose default editor cannot display a unified diff. + */ +public class NonTextTestEditor extends EditorPart { + + public static final String ID = "org.eclipse.compare.tests.nonTextEditor"; //$NON-NLS-1$ + + public static final String EXTENSION = "nontexteditortest"; //$NON-NLS-1$ + + @Override + public void doSave(IProgressMonitor monitor) { + // nothing to save + } + + @Override + public void doSaveAs() { + // saving as is not allowed + } + + @Override + public void init(IEditorSite site, IEditorInput input) { + setSite(site); + setInput(input); + } + + @Override + public boolean isDirty() { + return false; + } + + @Override + public boolean isSaveAsAllowed() { + return false; + } + + @Override + public void createPartControl(Composite parent) { + new Label(parent, SWT.NONE).setText("not a text editor"); //$NON-NLS-1$ + } + + @Override + public void setFocus() { + // no focusable content + } +} diff --git a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java index 88388f1057d..56e42656375 100644 --- a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java +++ b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java @@ -19,6 +19,8 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -56,11 +58,15 @@ import org.eclipse.jface.text.source.IAnnotationModel; import org.eclipse.swt.graphics.Image; import org.eclipse.swt.widgets.Display; +import org.eclipse.ui.IEditorDescriptor; import org.eclipse.ui.IEditorInput; import org.eclipse.ui.IEditorPart; +import org.eclipse.ui.IEditorReference; import org.eclipse.ui.IFileEditorInput; import org.eclipse.ui.IWorkbenchPage; import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.ide.IDE; +import org.eclipse.ui.part.FileEditorInput; import org.eclipse.ui.texteditor.IDocumentProvider; import org.eclipse.ui.texteditor.ITextEditor; import org.junit.jupiter.api.AfterEach; @@ -286,6 +292,68 @@ public void testClassicEditorOpensWhenInputDoesNotQualify() { "an input without shared document adapters must fall back to the compare editor"); //$NON-NLS-1$ } + /** + * The unified diff opens the editor before it knows whether the diff can be + * applied. When the registered editor turns out not to be a text editor, the + * classic compare editor takes over, and the editor opened a moment earlier + * must not stay behind as a second editor on the same file. + */ + @Test + public void testNoEditorIsLeftBehindWhenTheUnifiedDiffFallsBack() throws Exception { + IFile left = createFile("left." + NonTextTestEditor.EXTENSION, "alpha\nbravo\ncharlie\n"); //$NON-NLS-1$ //$NON-NLS-2$ + IFile right = createFile("right." + NonTextTestEditor.EXTENSION, "alpha\nBRAVO\ncharlie\n"); //$NON-NLS-1$ //$NON-NLS-2$ + assertEquals(NonTextTestEditor.ID, defaultEditorIdFor(left), + "the test needs a file whose default editor is not a text editor"); //$NON-NLS-1$ + + RecordingCompareEditorInput input = new RecordingCompareEditorInput(new WorkspaceFileElement(left), + new WorkspaceFileElement(right)); + CompareUI.openCompareEditor(input); + pumpUntil(UnifiedDiffOpenTest::hasCompareEditor, "the classic compare editor did not take over"); //$NON-NLS-1$ + processQueuedEvents(); + + assertNull(activePage().findEditor(new FileEditorInput(left)), + "the editor opened for the unified diff must be closed when the compare editor takes over"); //$NON-NLS-1$ + assertEquals(1, activePage().getEditorReferences().length, + "the fallback must leave exactly one editor open"); //$NON-NLS-1$ + } + + /** + * An editor the user already had open is not the unified diff's to close, so it + * survives the fallback to the compare editor. + */ + @Test + public void testAlreadyOpenEditorSurvivesTheFallback() throws Exception { + IFile left = createFile("left." + NonTextTestEditor.EXTENSION, "alpha\nbravo\ncharlie\n"); //$NON-NLS-1$ //$NON-NLS-2$ + IFile right = createFile("right." + NonTextTestEditor.EXTENSION, "alpha\nBRAVO\ncharlie\n"); //$NON-NLS-1$ //$NON-NLS-2$ + IEditorPart preOpened = IDE.openEditor(activePage(), left, NonTextTestEditor.ID); + assertNotNull(preOpened, "the editor to be preserved did not open"); //$NON-NLS-1$ + + RecordingCompareEditorInput input = new RecordingCompareEditorInput(new WorkspaceFileElement(left), + new WorkspaceFileElement(right)); + CompareUI.openCompareEditor(input); + pumpUntil(UnifiedDiffOpenTest::hasCompareEditor, "the classic compare editor did not take over"); //$NON-NLS-1$ + processQueuedEvents(); + + assertSame(preOpened, activePage().findEditor(new FileEditorInput(left)), + "an editor the unified diff did not open must stay open"); //$NON-NLS-1$ + assertEquals(2, activePage().getEditorReferences().length, + "the pre-opened editor and the compare editor must both be open"); //$NON-NLS-1$ + } + + private static String defaultEditorIdFor(IFile file) { + IEditorDescriptor descriptor = PlatformUI.getWorkbench().getEditorRegistry().getDefaultEditor(file.getName()); + return descriptor == null ? null : descriptor.getId(); + } + + private static boolean hasCompareEditor() { + for (IEditorReference reference : activePage().getEditorReferences()) { + if (reference.getEditor(false) instanceof CompareEditor) { + return true; + } + } + return false; + } + private RecordingCompareEditorInput openQualifyingInput() throws CoreException { IFile left = createFile("left.txt", "alpha\nbravo\ncharlie\ndelta\n"); //$NON-NLS-1$ //$NON-NLS-2$ IFile right = createFile("right.txt", "alpha\nBRAVO\ncharlie\ndelta\n"); //$NON-NLS-1$ //$NON-NLS-2$