From 2ed603cc79551b7918b0e4a232ef9d5f12e37952 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Tue, 4 Aug 2026 19:09:45 +0200 Subject: [PATCH] Offer a switch to the unified diff in the compare editor The unified diff has a toolbar button to open the comparison side by side, but there was no way back: seeing the same comparison as a unified diff meant changing the preference and opening it again. The text merge viewer now contributes the counterpart action. It appears only when the comparison can actually be shown as a unified diff, which is decided from the structure of the already computed compare result without reading any content, so a comparison without a workspace file to sit on keeps its toolbar clean. Switching reuses the prepared input rather than comparing again; only the side that supplies the diff is read, and that happens in a job. Once the unified diff is up the compare editor is closed, so a switch leaves one editor rather than two. When the comparison has unsaved merge changes, closing prompts as usual. --- .../contentmergeviewer/TextMergeViewer.java | 29 ++++++ .../TextMergeViewerResources.properties | 4 + .../compare/internal/CompareUIPlugin.java | 89 +++++++++++++++++-- .../icons/full/elcl16/unifieddiff_co.svg | 7 ++ .../compare/tests/UnifiedDiffOpenTest.java | 77 ++++++++++++++++ 5 files changed, 199 insertions(+), 7 deletions(-) create mode 100644 team/bundles/org.eclipse.compare/icons/full/elcl16/unifieddiff_co.svg diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java index e9ecc883494..1339979b966 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java @@ -43,6 +43,7 @@ import java.util.ResourceBundle; import org.eclipse.compare.CompareConfiguration; +import org.eclipse.compare.CompareEditorInput; import org.eclipse.compare.CompareNavigator; import org.eclipse.compare.CompareUI; import org.eclipse.compare.ICompareNavigator; @@ -4196,6 +4197,34 @@ public void run() { new MergeSourceViewer[] { fLeft, fRight, fAncestor }, AbstractDecoratedTextEditorPreferenceConstants.EDITOR_LINE_NUMBER_RULER); fHandlerService.registerAction(toggleLineNumbersAction, ITextEditorActionDefinitionIds.LINENUMBER_TOGGLE); + + createShowUnifiedDiffItem(tbm); + } + + /** + * Adds the action that switches this side by side comparison over to the + * unified diff, the counterpart of the unified diff's action to open the + * comparison. Only added when the input can be shown as a unified diff at all. + */ + private void createShowUnifiedDiffItem(ToolBarManager tbm) { + if (!(getCompareConfiguration().getContainer() instanceof CompareEditorInput input) + || !CompareUIPlugin.canShowAsUnifiedDiff(input)) { + return; + } + Action showUnifiedDiff = new Action() { + @Override + public void run() { + CompareUIPlugin.getDefault().switchToUnifiedDiff(input, getWorkbenchPage(input)); + } + }; + Utilities.initAction(showUnifiedDiff, getResourceBundle(), "action.ShowUnifiedDiff."); //$NON-NLS-1$ + tbm.add(new Separator()); + tbm.add(new ActionContributionItem(showUnifiedDiff)); + } + + private static IWorkbenchPage getWorkbenchPage(CompareEditorInput input) { + IWorkbenchPart part = input.getWorkbenchPart(); + return part == null ? null : part.getSite().getPage(); } private void configureCompareFilterActions(Object input, Object ancestor, diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewerResources.properties b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewerResources.properties index a3187af4220..3f42ba937e8 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewerResources.properties +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewerResources.properties @@ -115,3 +115,7 @@ Editor.FindReplace.description=Find/Replace action.IgnoreWhiteSpace.label=&Ignore White Space action.IgnoreWhiteSpace.tooltip=Ignore White Space Where Applicable + +action.ShowUnifiedDiff.label=Show Unified Diff +action.ShowUnifiedDiff.tooltip=Show the comparison as a unified diff +action.ShowUnifiedDiff.image=unifieddiff_co.svg 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 d4dc5f85cfc..21e7927d4a7 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 @@ -855,7 +855,41 @@ private UnifiedDiffSource prepareUnifiedDiff(CompareEditorInput input, IProgress CompareUIPlugin.log(e); return null; } - if (!(input.getCompareResult() instanceof ICompareInput compareInput)) { + return unifiedDiffSourceOf(input); + } + + /** + * Collects what the unified diff needs from an input that has already been run. + * Reads the side that supplies the diff, so it must not be called on the UI + * thread. Returns null if the input cannot be shown as a unified + * diff. + */ + private UnifiedDiffSource unifiedDiffSourceOf(CompareEditorInput input) { + UnifiedDiffCandidate candidate = unifiedDiffCandidateOf(input); + if (candidate == null) { + return null; + } + // The other side supplies the diff source and is read here rather than on the + // UI thread. + return new UnifiedDiffSource(candidate.compareInput(), candidate.editorInput(), candidate.element(), + candidate.mode(), getSourceOf(candidate.diffSource())); + } + + /** + * Returns whether the result of the given input can be displayed as a unified + * diff. Answers from the structure of the compare result alone, without reading + * any content, so it is cheap enough for deciding whether to offer the switch. + */ + public static boolean canShowAsUnifiedDiff(CompareEditorInput input) { + return unifiedDiffCandidateOf(input) != null; + } + + /** + * Picks the side the unified diff sits on, without reading any content. + * Returns null if neither side qualifies. + */ + private static UnifiedDiffCandidate unifiedDiffCandidateOf(CompareEditorInput input) { + if (input == null || !(input.getCompareResult() instanceof ICompareInput compareInput)) { return null; } // A common ancestor (3-way input) is ignored; the unified diff renders a @@ -869,19 +903,60 @@ private UnifiedDiffSource prepareUnifiedDiff(CompareEditorInput input, IProgress return null; } // The overlay needs a workspace file to sit on; an editor opened on anything - // else, a revision for example, comes up empty. The other side supplies the - // diff source and is read here rather than on the UI thread. + // else, a revision for example, comes up empty. if (leftEditorInput instanceof IFileEditorInput) { - return new UnifiedDiffSource(compareInput, leftEditorInput, left, UnifiedDiffMode.REVERT_MODE, - getSourceOf(rightSource)); + return new UnifiedDiffCandidate(compareInput, leftEditorInput, left, UnifiedDiffMode.REVERT_MODE, + rightSource); } if (rightEditorInput instanceof IFileEditorInput) { - return new UnifiedDiffSource(compareInput, rightEditorInput, right, UnifiedDiffMode.OVERLAY_READ_ONLY_MODE, - getSourceOf(leftSource)); + return new UnifiedDiffCandidate(compareInput, rightEditorInput, right, + UnifiedDiffMode.OVERLAY_READ_ONLY_MODE, leftSource); } return null; } + /** + * Switches the compare editor showing the given input over to the unified diff. + * The compare editor is closed once the unified diff is up; when it cannot be + * shown, the compare editor is left as it is. + */ + public void switchToUnifiedDiff(final CompareEditorInput input, final IWorkbenchPage page) { + final IWorkbenchPage wpage = page != null ? page : getActivePage(); + if (wpage == null || !canShowAsUnifiedDiff(input)) { + return; + } + Job job = new Job(NLS.bind(CompareMessages.UnifiedDiff_preparing, input.getTitle())) { + @Override + protected IStatus run(IProgressMonitor monitor) { + // The input already ran, so only the diff source has to be read here. + UnifiedDiffSource source = unifiedDiffSourceOf(input); + if (source == null || monitor.isCanceled()) { + return Status.CANCEL_STATUS; + } + Display.getDefault().asyncExec(() -> { + IEditorPart compareEditor = wpage.findEditor(input); + if (openUnifiedDiff(source, input, wpage, null, true) && compareEditor != null) { + // Prompts when the merge has unsaved changes. + wpage.closeEditor(compareEditor, true); + } + }); + return Status.OK_STATUS; + } + + @Override + public boolean belongsTo(Object family) { + return family == input || input.belongsTo(family); + } + }; + job.setUser(true); + job.schedule(); + } + + /** The side the unified diff would sit on, before any content is read. */ + private static record UnifiedDiffCandidate(ICompareInput compareInput, IEditorInput editorInput, + ITypedElement element, UnifiedDiffMode mode, IStreamContentAccessor diffSource) { + } + private static IEditorInput documentKeyOf(ITypedElement element) { if (element == null) { return null; diff --git a/team/bundles/org.eclipse.compare/icons/full/elcl16/unifieddiff_co.svg b/team/bundles/org.eclipse.compare/icons/full/elcl16/unifieddiff_co.svg new file mode 100644 index 00000000000..47c052325f8 --- /dev/null +++ b/team/bundles/org.eclipse.compare/icons/full/elcl16/unifieddiff_co.svg @@ -0,0 +1,7 @@ + + + + + + + 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 56e42656375..f365962e9d3 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 @@ -28,6 +28,8 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.Iterator; +import java.util.Locale; +import java.util.ResourceBundle; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BooleanSupplier; @@ -39,9 +41,11 @@ import org.eclipse.compare.ISharedDocumentAdapter; import org.eclipse.compare.ITypedElement; import org.eclipse.compare.SharedDocumentAdapter; +import org.eclipse.compare.contentmergeviewer.TextMergeViewer; import org.eclipse.compare.internal.CompareEditor; import org.eclipse.compare.internal.ComparePreferencePage; import org.eclipse.compare.internal.CompareUIPlugin; +import org.eclipse.compare.internal.Utilities; import org.eclipse.compare.unifieddiff.internal.UnifiedDiffManager; import org.eclipse.compare.structuremergeviewer.DiffNode; import org.eclipse.core.resources.IFile; @@ -52,6 +56,7 @@ import org.eclipse.core.runtime.IAdaptable; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.Status; +import org.eclipse.jface.action.Action; import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.jface.text.IDocument; import org.eclipse.jface.text.source.Annotation; @@ -340,6 +345,78 @@ public void testAlreadyOpenEditorSurvivesTheFallback() throws Exception { "the pre-opened editor and the compare editor must both be open"); //$NON-NLS-1$ } + /** + * The switch is only offered for a comparison the unified diff can actually + * display, so the toolbar of an input without a workspace file stays clean. + */ + @Test + public void testOnlyAQualifyingInputCanBeShownAsUnifiedDiff() throws Exception { + RecordingCompareEditorInput qualifying = openClassicInput(); + assertTrue(CompareUIPlugin.canShowAsUnifiedDiff(qualifying), + "a workspace file comparison must offer the unified diff"); //$NON-NLS-1$ + + RecordingCompareEditorInput inMemory = new RecordingCompareEditorInput( + new InMemoryElement("left.txt", "alpha\nbravo\n"), //$NON-NLS-1$ //$NON-NLS-2$ + new InMemoryElement("right.txt", "alpha\nBRAVO\n")); //$NON-NLS-1$ //$NON-NLS-2$ + CompareUI.openCompareEditor(inMemory); + pumpUntil(() -> inMemory.getCompareResult() != null, "the in-memory input was not prepared"); //$NON-NLS-1$ + assertFalse(CompareUIPlugin.canShowAsUnifiedDiff(inMemory), + "an input without a workspace file must not offer the unified diff"); //$NON-NLS-1$ + } + + /** + * Switching a side by side comparison over to the unified diff must leave the + * unified diff behind, not both editors. + */ + @Test + public void testSwitchToUnifiedDiffReplacesTheCompareEditor() throws Exception { + RecordingCompareEditorInput input = openClassicInput(); + int prepareInputCountBeforeSwitch = input.prepareInputCount.get(); + + CompareUIPlugin.getDefault().switchToUnifiedDiff(input, activePage()); + pumpUntil(() -> activePage().getActiveEditor() instanceof ITextEditor, + "the unified diff editor did not open"); //$NON-NLS-1$ + + ITextEditor textEditor = assertInstanceOf(ITextEditor.class, activePage().getActiveEditor()); + IAnnotationModel model = textEditor.getDocumentProvider().getAnnotationModel(textEditor.getEditorInput()); + assertNotNull(model, "the unified diff editor must have an annotation model"); //$NON-NLS-1$ + pumpUntil(() -> hasUnifiedDiffAnnotation(model), "unified diff annotations did not appear"); //$NON-NLS-1$ + + assertFalse(hasCompareEditor(), "the compare editor must close when the unified diff takes over"); //$NON-NLS-1$ + assertEquals(1, activePage().getEditorReferences().length, + "switching must leave exactly one editor open"); //$NON-NLS-1$ + assertEquals(prepareInputCountBeforeSwitch, input.prepareInputCount.get(), + "switching must reuse the already prepared input"); //$NON-NLS-1$ + } + + /** A missing resource key would leave the toolbar button blank. */ + @Test + public void testShowUnifiedDiffActionIsFullyDescribed() { + ResourceBundle bundle = ResourceBundle.getBundle("org.eclipse.compare.contentmergeviewer.TextMergeViewerResources", //$NON-NLS-1$ + Locale.getDefault(), TextMergeViewer.class.getClassLoader()); + Action action = new Action() { + // nothing to run, only the presentation is inspected + }; + Utilities.initAction(action, bundle, "action.ShowUnifiedDiff."); //$NON-NLS-1$ + + assertEquals("Show Unified Diff", action.getText(), "the action needs a label"); //$NON-NLS-1$ //$NON-NLS-2$ + assertNotNull(action.getToolTipText(), "the action needs a tooltip"); //$NON-NLS-1$ + assertNotNull(action.getImageDescriptor(), "the action needs an icon"); //$NON-NLS-1$ + } + + /** Opens the qualifying input in the classic compare editor. */ + private RecordingCompareEditorInput openClassicInput() throws CoreException { + store().setValue(ComparePreferencePage.UNIFIED_DIFF, false); + 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$ + RecordingCompareEditorInput input = new RecordingCompareEditorInput(new WorkspaceFileElement(left), + new WorkspaceFileElement(right)); + CompareUI.openCompareEditor(input); + pumpUntil(UnifiedDiffOpenTest::hasCompareEditor, "the compare editor did not open"); //$NON-NLS-1$ + pumpUntil(() -> input.getCompareResult() != null, "the compare input was not prepared"); //$NON-NLS-1$ + return input; + } + private static String defaultEditorIdFor(IFile file) { IEditorDescriptor descriptor = PlatformUI.getWorkbench().getEditorRegistry().getDefaultEditor(file.getName()); return descriptor == null ? null : descriptor.getId();