Skip to content

Offer a switch to the unified diff in the compare editor - #2854

Open
vogella wants to merge 1 commit into
eclipse-platform:masterfrom
vogella:compare-switch-to-unified-diff
Open

Offer a switch to the unified diff in the compare editor#2854
vogella wants to merge 1 commit into
eclipse-platform:masterfrom
vogella:compare-switch-to-unified-diff

Conversation

@vogella

@vogella vogella commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

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, so the two views can be switched in both directions.

The button appears only when the comparison can actually be shown as a unified diff. That 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, and only the side that supplies the diff is read, in a job.

Once the unified diff is up the compare editor is closed, so a switch leaves one editor rather than two. That is deliberately not symmetric with the other direction, which leaves the text editor open; a text editor is a normal editor the user may still want, while a compare editor that has been replaced is just clutter. Worth a second opinion, which is one reason this is a draft.

Draft, and stacked on #2853: the fallback path relies on that PR closing the editor it opened. The icon is a placeholder I drew and would benefit from a real one.

Contributes to #2795

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Test Results

    54 files  ±0      54 suites  ±0   55m 24s ⏱️ - 3m 30s
 4 710 tests +3   4 688 ✅ +3   22 💤 ±0  0 ❌ ±0 
12 021 runs  +9  11 868 ✅ +9  153 💤 ±0  0 ❌ ±0 

Results for commit 2ed603c. ± Comparison against base commit 1cc9ae4.

♻️ This comment has been updated with latest results.

@vogella
vogella force-pushed the compare-switch-to-unified-diff branch from 77161b0 to b1276b1 Compare August 6, 2026 10:06
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.
@vogella
vogella force-pushed the compare-switch-to-unified-diff branch from b1276b1 to 2ed603c Compare August 7, 2026 12:08
@vogella
vogella marked this pull request as ready for review August 10, 2026 08:30
@vogella
vogella requested a lite review from Copilot August 10, 2026 08:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a bidirectional view switch between the classic side-by-side compare editor and the unified diff view, allowing users to move back to unified diff without changing preferences/reopening the editor, while reusing the already-prepared compare input.

Changes:

  • Introduces a “Show Unified Diff” toolbar action in TextMergeViewer (only when the prepared compare result qualifies for unified diff).
  • Refactors unified-diff preparation to extract a reusable “candidate/source” from an already-prepared CompareEditorInput, and adds switchToUnifiedDiff(...) to open unified diff and close the compare editor.
  • Adds regression tests covering eligibility, editor replacement behavior, and action resource completeness; adds a new toolbar icon.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/UnifiedDiffOpenTest.java Adds tests for unified-diff eligibility, switching behavior, and resource completeness.
team/bundles/org.eclipse.compare/icons/full/elcl16/unifieddiff_co.svg Adds an icon for the new toolbar action.
team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/CompareUIPlugin.java Refactors unified diff source extraction and adds the switch-to-unified-diff job + editor-closing behavior.
team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewerResources.properties Adds label/tooltip/image keys for the new “Show Unified Diff” action.
team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java Contributes the new toolbar action to switch from side-by-side to unified diff when applicable.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +392 to +405
/** 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$
}
Comment on lines +936 to +942
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);
}
});
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.

2 participants