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 {