Show number of differences in the Compare editor #504

[lathapatil]

Show number of differences (Ex: 6 Differences) in the toolbar of compare editor
which matches "Change markers" next to the scroll bar in compare editor

Fixes #504

Tested only in Linux and Windows OS

[github-actions]

Test Results

     594 files  ±0       594 suites  ±0   37m 46s ⏱️ - 2m 35s
  3 863 tests +1    3 842 ✔️ +1    21 💤 ±0  0 ❌ ±0 
12 195 runs  +3  12 037 ✔️ +3  158 💤 ±0  0 ❌ ±0 

Results for commit 6108837. ± Comparison against base commit 07198c4.

♻️ This comment has been updated with latest results.

[iloveeclipse]

@lathapatil : could you please avoid merge commits? We don't allow them to be merged into master.

[iloveeclipse]

@lathapatil : you can see that you have now 3 commits in total, with two of them being merge commits.

Please locally (not in github, but in your IDE) create new branch based on latest master, cherry pick a single commit with your changes and force push to Enhancements/504_Show_differences_in_compareEditor branch.

[lathapatil]

@lathapatil : could you please avoid merge commits? We don't allow them to be merged into master.

I do realize now that "Update branch" button in this PR is actually doing merge commits and instead I should have used Sync fork and pull with rebase in my local then force pushed the branch.

[lathapatil]

@lathapatil : you can see that you have now 3 commits in total, with two of them being merge commits.

Please locally (not in github, but in your IDE) create new branch based on latest master, cherry pick a single commit with your changes and force push to Enhancements/504_Show_differences_in_compareEditor branch.

Thanks for these steps , I was about to ask for them as how to make it correct .

[lathapatil]

@iloveeclipse Could you please verify if commits are fine now and also review the PR ?

[iloveeclipse]

Commit looks good, but there is one related test fail, see https://ci.eclipse.org/platform/job/eclipse.platform/job/PR-731/4/testReport/junit/org.eclipse.compare.tests/TextMergeViewerTest/testToolbarLabelContribution/

java.lang.NullPointerException: Cannot invoke "org.eclipse.swt.widgets.Label.getText()" because the return value of "org.eclipse.compare.LabelContributionItem.getToolbarLabel()" is null
	at org.eclipse.compare.tests.TextMergeViewerTest.runInDialogWithToolbarDiffLabel(TextMergeViewerTest.java:530)
	at org.eclipse.compare.tests.TextMergeViewerTest.testToolbarLabelContribution(TextMergeViewerTest.java:501)

[lathapatil]

continuous-integration/jenkins/pr-head

one related test fail,

Actually this testcase is not failing in my local workspace . I am wondering how should I debug this and fix ? Can you suggest further on how do I get this setup where the testcase is failing..

[iloveeclipse]

Actually this testcase is not failing in my local workspace .

I can reproduce it. I'm on RHEL 7, Java 17, latest platform SDK build. What is your config?

I am wondering how should I debug this and fix ? Can you suggest further on how do I get this setup where the testcase is failing..

Looking at the code, it calls some tasks asynchronously, so I assume you could get the test failing on a different system / with more/less cores enabled (have no idea about your setup).

Anyway, please apply the patch here and the test should pass:

diff --git a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java
index a513593..ab21bc2 100644
--- a/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java
+++ b/team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java
@@ -526,11 +526,9 @@
 
-			IContributionItem contributionItem = toolbarManager.find("DiffCount");
-				assertNotNull(contributionItem);
-				LabelContributionItem labelContributionItem=(LabelContributionItem) contributionItem;
-				assertTrue(labelContributionItem.getToolbarLabel().getText().equals("7 Differences"));
-		try {
-			runnable.run();
-		} catch (WrappedException e) {
-			e.throwException();
-		}
+		processQueuedEvents();
+
+		IContributionItem contributionItem = toolbarManager.find("DiffCount");
+		assertNotNull(contributionItem);
+		LabelContributionItem labelContributionItem=(LabelContributionItem) contributionItem;
+		assertTrue(labelContributionItem.getToolbarLabel().getText().equals("7 Differences"));
+		runnable.run();
 		dialog.close();
@@ -539,2 +537,8 @@
 
+	private void processQueuedEvents() {
+		while (Display.getCurrent().readAndDispatch()) {
+			// Process all the events in the queue
+		}
+	}
+
 	private void runInDialogWithPartioner(Object input, Runnable runnable, final CompareConfiguration cc) throws Exception {

[lathapatil]

I can reproduce it. I'm on RHEL 7, Java 17, latest platform SDK build. What is your config?

Thanks for identifying the issue further and the fix .

I am working in Windows 10 , Java 17 and I do realize that testcase is passing in below platform build

Eclipse Platform 4.30.0.v20230927-1800

When I updated yesterday to latest one , it is failing now with the same error

Eclipse IDE for Eclipse Committers (includes Incubating components)
Version: 2023-12 M1 (4.30.0 M1)
Build id: 20231005-1720

[iloveeclipse]

I've quickly tested the feature. In general, works a s expected, but it has a bug.

1. The label is not recomputed if the diff config changes.
2. The label is incorrect for zero changes - it says 0 Difference

To reproduce:

1. commit a change that only contains some spaces or tabs added.
2. enable "ignore whitespace"
   
3. Diff - editor opens, shows 0 Difference
   
4. Toggle "Ignore whitespace" button on the main toolbar on/off - the label is unchanged but should reflect the diff state

[lathapatil]

3. Diff - editor opens, shows 0 Difference

@iloveeclipse Thanks for the detailed test .
What is the expected display of the label here ? "No changes" or no Display is required when 0 difference is there ?

[iloveeclipse]

What is the expected display of the label here ? "No changes" or no Display is required when 0 difference is there ?

"No changes" sounds good.

[lathapatil]

4. Toggle "Ignore whitespace" button on the main toolbar on/off - the label is unchanged but should reflect the diff state

I will check and fix this bug !

[iloveeclipse]

I will check and fix this bug !

Please also note that one can do edits in the compare editor and change number of diffs without triggering "whitespace" button.

[lathapatil]

I will check and fix this bug !

Please also note that one can do edits in the compare editor and change number of diffs without triggering "whitespace" button.

If I am not wrong this (Edit in compare editor and save which updates diff Label) is working fine. Only on trigger of "Ignore white space" Label is not getting updated

[iloveeclipse]

Test fail due eclipse-platform/eclipse.platform.releng.aggregator#1463 and eclipse-equinox/equinox#382.

Once that is sorted out, we can rebase & see if that helps.

[iloveeclipse]

The test fails as before, for same reason.
https://ci.eclipse.org/platform/job/eclipse.platform/job/PR-731/7/testReport/org.eclipse.compare.tests/TextMergeViewerTest/testToolbarLabelContribution/

Please try to adopt test as proposed above.

[lathapatil]

I will check and fix this bug !

Please also note that one can do edits in the compare editor and change number of diffs without triggering "whitespace" button.

@iloveeclipse I have fixed the points received in the review . Let me know if anything is still not working out in this feature.

[iloveeclipse]

The errors are coming from tycho eclipse-tycho/tycho#3019

[lathapatil]

The errors are coming from tycho eclipse-tycho/tycho#3019

Is this issue resolved ? If yes, how can I retrigger the build to see latest status?

[HeikoKlare]

It has been resolved in Tycho, but the Tycho version used in the builds here has not been updated yet. Builds should work again once eclipse-platform/eclipse.platform.releng.aggregator#1530 is merged (or maybe once we adapt the project configurations so they conform to Tycho 4.0.4 requirements).

[HeikoKlare]

@lathapatil The build can now be retriggered. You only have to manually rebase your changes on current master branch, as the version bump for org.eclipse.compare in this PR became conflicting with the change in the master branch.

[iloveeclipse]

Thanks, I've tested locally, all the changes are now properly reflected in the diff label.

I've reviewed the rest, please fix the two issues I've commented on and we are ready to merge after that.

[iloveeclipse]

Thanks for your patience & contribution. Looks good now.