Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for "editor" content types for compare editor #1277

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

iloveeclipse
Copy link
Member

@iloveeclipse iloveeclipse commented Mar 29, 2024

Extend contentMergeViewers extension point: add an attribute to consider "linked" editor content type associations. The code checks if contentMergeViewers contribution refers to such "linked" editor id in extension, and check if editor supports other content types. In case there are more content types supported by same "linked" editor, these would be automatically considered as valid contentTypes for the contentMergeViewers.

Updated CompareViewerSwitchingPane to consider more precise title calculations for ContentMergeViewer instances (where concrete type info is available after instantiation of a particular viewer).

Fixes eclipse-platform/eclipse.platform.ui#1747

iloveeclipse added a commit to iloveeclipse/eclipse.platform.ui that referenced this pull request Mar 29, 2024
This allows GenericEditorMergeViewer be used by compare editor for
content types associated with the generic editor.

Additionally implemented getTitle() to provide more specific compare
editor description, so instead of default "Text Compare" one would see
"XML Compare" etc. This avoids the problem of duplicated "Text Compare"
options shown for different compare viewers (default one for plain text
and the one for generic editor).

See eclipse-platform#1747

See eclipse-platform/eclipse.platform#1277
Copy link
Contributor

github-actions bot commented Mar 29, 2024

Test Results

   639 files  ±0     639 suites  ±0   41m 30s ⏱️ + 1m 27s
 3 946 tests ±0   3 924 ✅ ±0   22 💤 ±0  0 ❌ ±0 
12 444 runs  ±0  12 283 ✅ ±0  161 💤 ±0  0 ❌ ±0 

Results for commit 0e954ac. ± Comparison against base commit dbb36aa.

♻️ This comment has been updated with latest results.

@iloveeclipse
Copy link
Member Author

Tests run: 3, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 0.256 s <<< FAILURE! -- in AllCompareTests org.eclipse.compare.tests.PatchUITest
org.eclipse.compare.tests.PatchUITest.testApplyClipboardPatch -- Time elapsed: 0.126 s <<< ERROR!
java.lang.NullPointerException: Cannot invoke "java.util.HashMap.values()" because "this.fIdMap" is null
	at org.eclipse.compare.internal.CompareUIPlugin$CompareRegistry.getAll(CompareUIPlugin.java:201)
	at org.eclipse.compare.internal.CompareUIPlugin.findEditorLinkedDescriptors(CompareUIPlugin.java:1126)
	at org.eclipse.compare.internal.CompareUIPlugin.findContentViewerDescriptor(CompareUIPlugin.java:1066)
	at org.eclipse.compare.internal.CompareUIPlugin.findContentViewer(CompareUIPlugin.java:1172)
	at org.eclipse.compare.CompareUI.findContentViewer(CompareUI.java:338)
	at org.eclipse.compare.CompareEditorInput.findContentViewer(CompareEditorInput.java:916)
	at org.eclipse.compare.internal.CompareContentViewerSwitchingPane.getViewer(CompareContentViewerSwitchingPane.java:99)
	at org.eclipse.compare.CompareViewerSwitchingPane.setInput(CompareViewerSwitchingPane.java:258)
	at org.eclipse.compare.internal.CompareContentViewerSwitchingPane.setInput(CompareContentViewerSwitchingPane.java:202)
	at org.eclipse.compare.CompareEditorInput.internalSetContentPaneInput(CompareEditorInput.java:806)
	at org.eclipse.compare.CompareEditorInput.lambda$8(CompareEditorInput.java:747)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67)
	at org.eclipse.compare.CompareEditorInput.feed1(CompareEditorInput.java:742)
	at org.eclipse.compare.CompareEditorInput.feedInput(CompareEditorInput.java:720)
	at org.eclipse.compare.CompareEditorInput.createContents(CompareEditorInput.java:543)
	at org.eclipse.compare.internal.patch.PreviewPatchPage2.createControl(PreviewPatchPage2.java:144)
	at org.eclipse.jface.wizard.Wizard.createPageControls(Wizard.java:180)
	at org.eclipse.jface.wizard.WizardDialog.createPageControls(WizardDialog.java:744)
	at org.eclipse.jface.wizard.WizardDialog.createContents(WizardDialog.java:637)
	at org.eclipse.jface.window.Window.create(Window.java:431)
	at org.eclipse.jface.dialogs.Dialog.create(Dialog.java:1092)

Extend `contentMergeViewers` extension point: add an attribute to
consider "linked" editor content type associations. The code checks if
contentMergeViewers contribution refers to such "linked" editor id in
extension, and check if editor supports other content types. In case
there are more content types supported by same "linked" editor, these
would be automatically considered as valid contentTypes for the
contentMergeViewers.

Updated CompareViewerSwitchingPane to consider more precise title
calculations for ContentMergeViewer instances (where concrete type info
is available after instantiation of a particular viewer).

Fixes eclipse-platform/eclipse.platform.ui#1747
@iloveeclipse iloveeclipse merged commit 6de5471 into eclipse-platform:master Apr 3, 2024
16 checks passed
@iloveeclipse iloveeclipse deleted the issue_1747 branch April 3, 2024 15:58
iloveeclipse added a commit to eclipse-platform/eclipse.platform.ui that referenced this pull request Apr 3, 2024
This allows GenericEditorMergeViewer be used by compare editor for
content types associated with the generic editor.

Additionally implemented getTitle() to provide more specific compare
editor description, so instead of default "Text Compare" one would see
"XML Compare" etc. This avoids the problem of duplicated "Text Compare"
options shown for different compare viewers (default one for plain text
and the one for generic editor).

See #1747

See eclipse-platform/eclipse.platform#1277
@akurtakov
Copy link
Member

@iloveeclipse
Copy link
Member Author

I'm on it, I have a fix but have meetings all the day.

iloveeclipse added a commit to iloveeclipse/eclipse.platform that referenced this pull request Apr 4, 2024
TextMergeViewer was expected by SaveableCompareEditorInputTest but more
specific implementation (subclass of it) - GenericEditorMergeViewer was
seen by the test after changes made to allow more specific viewer to
"win" in the compare editor for text content.

See eclipse-platform/eclipse.platform.ui#1747
See eclipse-platform#1277
@iloveeclipse
Copy link
Member Author

See #1283

akurtakov pushed a commit that referenced this pull request Apr 4, 2024
TextMergeViewer was expected by SaveableCompareEditorInputTest but more
specific implementation (subclass of it) - GenericEditorMergeViewer was
seen by the test after changes made to allow more specific viewer to
"win" in the compare editor for text content.

See eclipse-platform/eclipse.platform.ui#1747
See #1277
iloveeclipse added a commit to iloveeclipse/eclipse.platform that referenced this pull request Jul 2, 2024
As noted on PR eclipse-platform#1277, the code looks weird and I can't explain what I
smoke while writing it. Fixed the code to do what actually was meant.
This is mostly paranoia code, as usually input.getName() is not null.

See eclipse-platform#1277
See eclipse-platform/eclipse.platform.ui#1747
iloveeclipse added a commit that referenced this pull request Jul 2, 2024
As noted on PR #1277, the code looks weird and I can't explain what I
smoke while writing it. Fixed the code to do what actually was meant.
This is mostly paranoia code, as usually input.getName() is not null.

See #1277
See eclipse-platform/eclipse.platform.ui#1747
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.

[RFE] Add support for "editor" content types for compare editor / contentViewers extension
3 participants