From b9a3d343e5b996103250fe31b234716e6a1fc268 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 13 Aug 2026 14:08:21 +0200 Subject: [PATCH] Show the unified diff for a file that is gone from the workspace Comparing a file that was deleted in the workspace, its state against the git index for example, opened the classic compare editor. Neither side is a workspace file then, so there was nothing to overlay the diff onto. The version that still exists is shown read-only instead, with its whole content marked as removed. Its document key is a storage editor input, and StorageDocumentProvider supplies no annotation model for it, which is the real reason such an editor came up without any diff: the annotations had nowhere to go. The unified diff now installs a model on the source viewer when the document provider has none, and falls back to the classic compare editor when the version comes up without content at all. Two sides that both carry content, staged changes for example, keep opening the classic compare editor, because neither of them is the state on disk. --- .../compare/internal/CompareUIPlugin.java | 53 ++++- .../internal/HideAllDiffsRunnable.java | 2 +- .../internal/UnifiedDiffManager.java | 35 ++- .../compare/tests/UnifiedDiffOpenTest.java | 201 +++++++++++++++++- 4 files changed, 278 insertions(+), 13 deletions(-) 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 1e7d36c9ada..f3df9ab744a 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 @@ -88,6 +88,7 @@ import org.eclipse.jface.operation.IRunnableContext; import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.jface.resource.ImageDescriptor; +import org.eclipse.jface.text.IDocument; import org.eclipse.jface.util.IPropertyChangeListener; import org.eclipse.jface.viewers.Viewer; import org.eclipse.osgi.service.debug.DebugOptions; @@ -720,6 +721,12 @@ private boolean openUnifiedDiff(UnifiedDiffSource source, CompareEditorInput inp closeIfOpenedHere(wpage, openedHere); return false; } + // A side that is not a workspace file, the version shown for a deleted file + // for example, is only worth overlaying when its content really loaded. + if (!(source.editorInput() instanceof IFileEditorInput) && !hasContent(textEditor)) { + closeIfOpenedHere(wpage, openedHere); + return false; + } Action openTwoWayCompare = createOpenTwoWayCompareAction(input, page, editor, activate, textEditor); IStatus status = UnifiedDiff.create(textEditor, source.diffSource(), source.mode()) .additionalActions(Arrays.asList(openTwoWayCompare)) @@ -744,6 +751,11 @@ private boolean openUnifiedDiff(UnifiedDiffSource source, CompareEditorInput inp return false; } + private static boolean hasContent(ITextEditor editor) { + IDocument document = editor.getDocumentProvider().getDocument(editor.getEditorInput()); + return document != null && document.getLength() > 0; + } + private static void closeIfOpenedHere(IWorkbenchPage page, IEditorPart editor) { if (editor != null) { // Nothing here writes to the document, so there is nothing to save. @@ -897,17 +909,31 @@ private static UnifiedDiffCandidate unifiedDiffCandidateOf(CompareEditorInput in // plain left-vs-right overlay. ITypedElement left = compareInput.getLeft(); ITypedElement right = compareInput.getRight(); - // Only the side shown in the editor has to be a workspace file; an editor - // opened on anything else, a revision for example, comes up empty. The other - // side merely supplies the diff source and may be missing entirely, as for a - // newly added file. - if (documentKeyOf(left) instanceof IFileEditorInput leftEditorInput) { + IEditorInput leftEditorInput = documentKeyOf(left); + IEditorInput rightEditorInput = documentKeyOf(right); + // The side shown in the editor is preferably a workspace file; an editor opened + // on anything else, a revision for example, comes up empty. The other side + // merely supplies the diff source and may be missing entirely, as for a newly + // added file. + if (leftEditorInput instanceof IFileEditorInput) { return new UnifiedDiffCandidate(compareInput, leftEditorInput, left, UnifiedDiffMode.REVERT_MODE, right); } - if (documentKeyOf(right) instanceof IFileEditorInput rightEditorInput) { + if (rightEditorInput instanceof IFileEditorInput) { + return new UnifiedDiffCandidate(compareInput, rightEditorInput, right, + UnifiedDiffMode.OVERLAY_READ_ONLY_MODE, left); + } + // A deleted file leaves no workspace file to overlay. The version that still + // exists is then shown read-only, with its whole content marked as removed. + // Two sides that both exist are left to the classic compare editor, because + // neither of them is the state on disk. + if (rightEditorInput != null && isAbsent(left)) { return new UnifiedDiffCandidate(compareInput, rightEditorInput, right, UnifiedDiffMode.OVERLAY_READ_ONLY_MODE, left); } + if (leftEditorInput != null && isAbsent(right)) { + return new UnifiedDiffCandidate(compareInput, leftEditorInput, left, + UnifiedDiffMode.OVERLAY_READ_ONLY_MODE, right); + } return null; } @@ -953,6 +979,21 @@ private static record UnifiedDiffCandidate(ICompareInput compareInput, IEditorIn ITypedElement element, UnifiedDiffMode mode, ITypedElement diffSource) { } + /** + * Returns whether the given side does not exist, as the workspace file of a + * deleted resource or the previous version of an added file. Answered from the + * element alone, so that picking a candidate stays free of content reads. + */ + private static boolean isAbsent(ITypedElement element) { + // null, or an element that only names the file + if (!(element instanceof IStreamContentAccessor)) { + return true; + } + IResource resource = element instanceof IResourceProvider provider ? provider.getResource() + : Adapters.adapt(element, IResource.class); + return resource != null && !resource.exists(); + } + private static IEditorInput documentKeyOf(ITypedElement element) { if (element == null) { return null; diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/HideAllDiffsRunnable.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/HideAllDiffsRunnable.java index daf1eb91ba1..8b526d46efc 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/HideAllDiffsRunnable.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/HideAllDiffsRunnable.java @@ -42,7 +42,7 @@ public HideAllDiffsRunnable(ITextViewer tv, IAnnotationModel model) { public HideAllDiffsRunnable(ITextEditor textEditor) { this.tv = textEditor.getAdapter(ITextViewer.class); - this.model = textEditor.getDocumentProvider().getAnnotationModel(textEditor.getEditorInput()); + this.model = UnifiedDiffManager.annotationModelOf(textEditor); } public String getLabel() { diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java index a11c1e0cf82..cb5a6012edd 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java @@ -63,11 +63,13 @@ import org.eclipse.jface.text.Position; import org.eclipse.jface.text.codemining.ICodeMining; import org.eclipse.jface.text.source.Annotation; +import org.eclipse.jface.text.source.AnnotationModel; import org.eclipse.jface.text.source.AnnotationModelEvent; import org.eclipse.jface.text.source.IAnnotationModel; import org.eclipse.jface.text.source.IAnnotationModelExtension; import org.eclipse.jface.text.source.IAnnotationModelListener; import org.eclipse.jface.text.source.IAnnotationModelListenerExtension; +import org.eclipse.jface.text.source.ISourceViewer; import org.eclipse.jface.text.source.ISourceViewerExtension5; import org.eclipse.jface.text.source.inlined.AbstractInlinedAnnotation; import org.eclipse.jface.text.source.projection.ProjectionViewer; @@ -130,6 +132,34 @@ public static List get(ITextViewer viewer) { return diffsByViewer.get(viewer); } + /** + * Returns the annotation model the diffs of the given editor live in, or + * null when it has none. The document provider of an editor on + * something other than a workspace file, a revision for example, supplies no + * model, so the one installed on the viewer is used. + */ + public static IAnnotationModel annotationModelOf(ITextEditor editor) { + IAnnotationModel model = editor.getDocumentProvider().getAnnotationModel(editor.getEditorInput()); + if (model != null) { + return model; + } + return editor.getAdapter(ITextViewer.class) instanceof ISourceViewer sourceViewer + ? sourceViewer.getAnnotationModel() + : null; + } + + /** + * Installs an annotation model on the viewer of an editor whose document + * provider has none, so that the diffs have somewhere to be shown. + */ + private static IAnnotationModel installAnnotationModel(ITextViewer viewer, IDocument document) { + if (document == null || !(viewer instanceof ISourceViewer sourceViewer)) { + return null; + } + sourceViewer.setDocument(document, new AnnotationModel()); + return sourceViewer.getAnnotationModel(); + } + public static IStatus open(ITextEditor editor, String source, UnifiedDiffMode mode, List additionalActions, TokenComparatorFactory tokenComparatorFactory, IgnoreWhitespaceContributorFactory ignoreWhitespaceContributorFactory, boolean ignoreWhiteSpace) { @@ -137,13 +167,14 @@ public static IStatus open(ITextEditor editor, String source, UnifiedDiffMode mo if (viewer instanceof ProjectionViewer pv) { pv.doOperation(ProjectionViewer.EXPAND_ALL); } - IAnnotationModel model = editor.getDocumentProvider().getAnnotationModel(editor.getEditorInput()); + IDocument leftDocument = editor.getDocumentProvider().getDocument(editor.getEditorInput()); + IAnnotationModel editorModel = annotationModelOf(editor); + IAnnotationModel model = editorModel != null ? editorModel : installAnnotationModel(viewer, leftDocument); if (model == null) { return Status.CANCEL_STATUS; } clearAll(viewer, model); - IDocument leftDocument = editor.getDocumentProvider().getDocument(editor.getEditorInput()); IDocument rightDocument = new Document(source); List unifiedDiffs; 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 6acfc1607a4..95329468bd1 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 @@ -51,13 +51,16 @@ import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; +import org.eclipse.core.resources.IStorage; import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IAdaptable; +import org.eclipse.core.runtime.IPath; 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.resource.ImageDescriptor; import org.eclipse.jface.text.IDocument; import org.eclipse.jface.text.source.Annotation; import org.eclipse.jface.text.source.IAnnotationModel; @@ -68,6 +71,8 @@ import org.eclipse.ui.IEditorPart; import org.eclipse.ui.IEditorReference; import org.eclipse.ui.IFileEditorInput; +import org.eclipse.ui.IPersistableElement; +import org.eclipse.ui.IStorageEditorInput; import org.eclipse.ui.IWorkbenchPage; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.ide.IDE; @@ -114,7 +119,8 @@ public void flushDocument(IDocumentProvider provider, IEditorInput documentKey, @Override public InputStream getContents() throws CoreException { - return file.getContents(); + // as LocalResourceTypedElement does for a resource that is gone + return file.exists() ? file.getContents() : null; } @Override @@ -216,6 +222,139 @@ public Image getImage() { } } + /** + * An element for a version of a file that is not in the workspace, as a version + * control system contributes it. Its document key is a storage editor input, so + * a text editor can show it read-only. + */ + private static final class RevisionElement implements ITypedElement, IEncodedStreamContentAccessor, IAdaptable { + + private final String name; + private final byte[] bytes; + private final IEditorInput documentKey; + + RevisionElement(String name, String content) { + this.name = name; + this.bytes = content.getBytes(StandardCharsets.UTF_8); + this.documentKey = new RevisionEditorInput(this); + } + + @Override + public InputStream getContents() { + return new ByteArrayInputStream(bytes); + } + + @Override + public String getCharset() { + return "UTF-8"; //$NON-NLS-1$ + } + + @Override + public String getName() { + return name; + } + + @Override + public String getType() { + return TEXT_TYPE; + } + + @Override + public Image getImage() { + return null; + } + + @Override + public T getAdapter(Class adapter) { + if (adapter == ISharedDocumentAdapter.class) { + return adapter.cast(new SharedDocumentAdapter() { + @Override + public IEditorInput getDocumentKey(Object element) { + return element == RevisionElement.this ? documentKey : null; + } + + @Override + public void flushDocument(IDocumentProvider provider, IEditorInput key, IDocument document, + boolean overwrite) { + // a revision is read-only + } + }); + } + return null; + } + } + + /** The read-only editor input of a {@link RevisionElement}. */ + private static final class RevisionEditorInput implements IStorageEditorInput { + + private final RevisionElement element; + + RevisionEditorInput(RevisionElement element) { + this.element = element; + } + + @Override + public IStorage getStorage() { + return new IStorage() { + + @Override + public InputStream getContents() { + return element.getContents(); + } + + @Override + public IPath getFullPath() { + return IPath.fromOSString(element.getName()); + } + + @Override + public String getName() { + return element.getName(); + } + + @Override + public boolean isReadOnly() { + return true; + } + + @Override + public T getAdapter(Class adapter) { + return null; + } + }; + } + + @Override + public boolean exists() { + return true; + } + + @Override + public ImageDescriptor getImageDescriptor() { + return null; + } + + @Override + public String getName() { + return element.getName(); + } + + @Override + public IPersistableElement getPersistable() { + return null; + } + + @Override + public String getToolTipText() { + return element.getName(); + } + + @Override + public T getAdapter(Class adapter) { + return null; + } + } + /** Records prepareInput invocations and the thread they ran on. */ private static final class RecordingCompareEditorInput extends CompareEditorInput { @@ -309,6 +448,54 @@ public void testUnifiedEditorOpensOnRightWhenLeftIsAbsent() throws Exception { assertUnifiedDiffEditor("right.txt"); //$NON-NLS-1$ } + /** + * A file deleted in the workspace has no file left to overlay, so the version + * that still exists is shown read-only with its content marked as removed. + */ + @Test + public void testUnifiedEditorOpensOnTheRevisionWhenTheWorkspaceFileIsDeleted() throws Exception { + IFile deleted = createFile("gone.txt", "alpha\nbravo\ncharlie\n"); //$NON-NLS-1$ //$NON-NLS-2$ + WorkspaceFileElement local = new WorkspaceFileElement(deleted); + deleted.delete(true, null); + + openInput(local, new RevisionElement("gone.txt", "alpha\nbravo\ncharlie\n")); //$NON-NLS-1$ //$NON-NLS-2$ + + ITextEditor textEditor = assertUnifiedDiffEditor(); + assertInstanceOf(IStorageEditorInput.class, textEditor.getEditorInput(), + "the unified diff must be shown on the version that still exists"); //$NON-NLS-1$ + } + + /** + * Two versions that both carry content, staged changes for example, keep opening + * the classic compare editor: neither of them is the state on disk. + */ + @Test + public void testClassicEditorOpensForTwoRevisions() { + openInput(new RevisionElement("staged.txt", "alpha\nbravo\n"), //$NON-NLS-1$ //$NON-NLS-2$ + new RevisionElement("staged.txt", "alpha\nBRAVO\n")); //$NON-NLS-1$ //$NON-NLS-2$ + + pumpUntil(UnifiedDiffOpenTest::hasCompareEditor, "the classic compare editor did not take over"); //$NON-NLS-1$ + } + + /** + * Nothing can be marked as removed when the version that is meant to be shown + * comes up without content, so the classic compare editor takes over and the + * editor opened for the unified diff is closed again. + */ + @Test + public void testClassicEditorOpensWhenTheRemainingRevisionIsEmpty() throws Exception { + IFile deleted = createFile("gone.txt", "alpha\nbravo\ncharlie\n"); //$NON-NLS-1$ //$NON-NLS-2$ + WorkspaceFileElement local = new WorkspaceFileElement(deleted); + deleted.delete(true, null); + + openInput(local, new RevisionElement("gone.txt", "")); //$NON-NLS-1$ //$NON-NLS-2$ + + pumpUntil(UnifiedDiffOpenTest::hasCompareEditor, "the classic compare editor did not take over"); //$NON-NLS-1$ + processQueuedEvents(); + assertEquals(1, activePage().getEditorReferences().length, + "the fallback must leave exactly one editor open"); //$NON-NLS-1$ + } + @Test public void testUnifiedOpenRunsPrepareInputOffUiThread() throws Exception { RecordingCompareEditorInput input = openQualifyingInput(); @@ -491,17 +678,23 @@ private RecordingCompareEditorInput openInput(ITypedElement left, ITypedElement /** Asserts that the unified diff is shown in a text editor on the given file. */ private static void assertUnifiedDiffEditor(String fileName) { + ITextEditor textEditor = assertUnifiedDiffEditor(); + IFileEditorInput editorInput = assertInstanceOf(IFileEditorInput.class, textEditor.getEditorInput()); + assertEquals(fileName, editorInput.getFile().getName()); + } + + /** Asserts that the active editor is a text editor showing a unified diff. */ + private static ITextEditor assertUnifiedDiffEditor() { IEditorPart editorPart = activePage().getActiveEditor(); assertNotNull(editorPart, "an editor must be open"); //$NON-NLS-1$ assertFalse(editorPart instanceof CompareEditor, "a qualifying input must open a text editor, not the compare editor"); //$NON-NLS-1$ ITextEditor textEditor = assertInstanceOf(ITextEditor.class, editorPart); - IFileEditorInput editorInput = assertInstanceOf(IFileEditorInput.class, textEditor.getEditorInput()); - assertEquals(fileName, editorInput.getFile().getName()); - IAnnotationModel model = textEditor.getDocumentProvider().getAnnotationModel(textEditor.getEditorInput()); + IAnnotationModel model = UnifiedDiffManager.annotationModelOf(textEditor); 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$ + return textEditor; } private IFile createFile(String name, String content) throws CoreException {