Community
Participate
Working Groups
No code mining when using Ctrl+End. Open a larger file, e.g. TextViewer and then press Ctrl+End ==> code minings are not shown.
Code minings are shown in TextViewer but after some time when the calculation is finished. I think it is a duplicate of bug 541441. *** This bug has been marked as a duplicate of bug 541441 ***
(In reply to Noopur Gupta from comment #1) > Code minings are shown in TextViewer but after some time when the > calculation is finished. I think it is a duplicate of bug 541441. > > *** This bug has been marked as a duplicate of bug 541441 *** Nope.
Yes, this is most strange, On some methods in between reference count come like getModelCoverage, addPainter after 7 minutes of opening this file. Is it so slow? Noopur, how much time did it take for you?
(In reply to Sarika Sinha from comment #3) > Noopur, how much time did it take for you? Steps that I tried: - Open TextViewer.java. - Press Ctrl+End. => I get the annotations on #setTabsToSpacesConverter, #ensureHyperlinkManagerInstalled etc. in approximately 8 seconds.
Tried again with having TextViewer java file locally, still on Ctr+End or even scrolling through the file slowly/fast, I see Code Minings selectively. Like I see on KeyPressed but not on mousep/down, After some time I see on ensureHyperlinkManagerInstalled but I never see on setTabsToSpacesConverter. @Angelo, can you explain the reason.
It's a bug of CodeMining that I'm trying to fix. The basic explanation is that inlined annotation (like codemining header annotation) checks if the annotation offset is in visibles lines area -> https://github.com/eclipse/eclipse.platform.text/blob/master/org.eclipse.jface.text/src/org/eclipse/jface/text/source/inlined/InlinedAnnotationDrawingStrategy.java#L42 The visibles lines uses a IViewportListener to update the start/end offset of visibles lines. The problem with Ctrl+End is that inlined annotations is drawn BEFORE the update of IViewportListener, that's why the annotation is considered as not in visible lines area. A fix that I have in my mind is to uses a paint listener like we have done with VisibleLinesTracker to track changes of visible lines for *RulerColumn. A clean solution is to reuse this tracker, but we cannot use it, since VisibleLinesTracker is only visible for org.eclipse.jface.text.source package and fix should be done org.eclipse.jface.text.source.inlined package (InlinedAnnotationSupport).
New Gerrit change created: https://git.eclipse.org/r/133055
Please see my gerrit patch at https://git.eclipse.org/r/#/c/133055/ which follows my idea with visible lines tracker. I know this gerrit patch will be refused since I have changed VisibleLinesTracker to public (class and track/untrack methods), but I wanted to open discussion, because I think more and more JFace Text should provide this tracker, which is used in several area. The benefit with this tracker is that compute of bottom index and offset is done ONCE time and not several times (for each classes which needs that like *RulerAnnotation and now InlinedAnnotationSupport). This gerrit patch should fix this issue with Ctrl+End BUT I don't know why the lines which are below mining to draw are cut -( I think it's a new bug with StyledText?
(In reply to Sarika Sinha from comment #5) > Tried again with having TextViewer java file locally, still on Ctr+End or > even scrolling through the file slowly/fast, I see Code Minings selectively. > Like I see on KeyPressed but not on mousep/down, After some time I see on > ensureHyperlinkManagerInstalled but I never see on setTabsToSpacesConverter. > > @Angelo, can you explain the reason.
Strange. Today it seems to work, though with delay towards 10s.
It works for also with Eclipse SDK Version: 2018-12 (4.10) Build id: I20181126-0940
(In reply to Sarika Sinha from comment #11) > It works for also with Eclipse SDK > > Version: 2018-12 (4.10) > Build id: I20181126-0940 works for me.
Created attachment 276713 [details] Inconsistent code minings Adding some use cases to test after the fix is available - 1. As seen in the attachment, Code Minings annotation are sometime very in consistent in display , Just showing references or empty line for annotation which is clickable. 2. In some scenarios, after Ctrl + End, Code Minings appear only if there is a click on the editor.
(In reply to Dani Megert from comment #10) > Strange. Today it seems to work, though with delay towards 10s. OK, I can consistently reproduce now. It depends on when I press Ctrl+End. Here it works: 1. Use Open Type to open TextViewer 2. Immediately press Ctrl+End ==> code minings appear When I now press Ctrl+Home the minings are not shown. Here it does not work: 1. Use Open Type to open TextViewer 2. Wait until the code minings are visible (also the one above class TextViewer This takes about 5s 2. Press Ctrl+End ==> no code minings When I now click somewhere, the minings appear.
(In reply to Dani Megert from comment #14) Looks like a listener is missing or not checking Ctrl+End and Ctrl+Home.
(In reply to Sarika Sinha from comment #13) > Created attachment 276713 [details] > Inconsistent code minings > > Adding some use cases to test after the fix is available - > 1. As seen in the attachment, Code Minings annotation are sometime very in > consistent in display , Just showing references or empty line for annotation > which is clickable. > 2. In some scenarios, after Ctrl + End, Code Minings appear only if there is > a click on the editor. This looks like a different bug. Please file a new bug report with steps to reproduce.
(In reply to Angelo ZERR from comment #8) > Please see my gerrit patch at https://git.eclipse.org/r/#/c/133055/ which > follows my idea with visible lines tracker. I know this gerrit patch will be > refused. It depends how you make it API. Let's discuss in the Gerrit itself.
> When I now press Ctrl+Home the minings are not shown. You mean https://git.eclipse.org/r/#/c/133055/ doesn't work?
(In reply to Angelo ZERR from comment #18) > > When I now press Ctrl+Home the minings are not shown. > > You mean https://git.eclipse.org/r/#/c/133055/ doesn't work? No, I didn't use the change as it is not yet "good". I only wanted to provide exact steps to reproduce it consistently. Comments about the change, e.g. whether it works or not, belong into the change and not into this bug report.
(In reply to Dani Megert from comment #15) > Looks like a listener is missing or not checking Ctrl+End and Ctrl+Home. This could very well be caused by https://www.eclipse.org/lists/platform-dev/msg01475.html
(In reply to Mickael Istria - away until Feb 14th from comment #20) > (In reply to Dani Megert from comment #15) > > Looks like a listener is missing or not checking Ctrl+End and Ctrl+Home. > > This could very well be caused by > https://www.eclipse.org/lists/platform-dev/msg01475.html Could, but I doubt it. This one is regarding Ctrl+End (makes no selection) not Shift+End (makes selection). Eric did not complain about Ctrl+Home which is the same as Ctrl+End but in the other direction. In addition, when pressing Ctrl+End right at the beginning it does work.
Mass change, please reset target if you still planning to fix this for 4.11.
> Here it works: > 1. Use Open Type to open TextViewer > 2. Immediately press Ctrl+End > ==> code minings appear > > When I now press Ctrl+Home the minings are not shown. It looks like I can reproduce this one. > Here it does not work: > 1. Use Open Type to open TextViewer > 2. Wait until the code minings are visible (also the one above class > TextViewer > This takes about 5s > 2. Press Ctrl+End > ==> no code minings I couldn't reproduce this one. Which line range is visible when you open the file and then when you hit Ctrl+End? It seems to me the set of visible lines can have an impact on the results.
(In reply to Mickael Istria from comment #23) > > 2. Wait until the code minings are visible (also the one above class This is very important. Better wait another few seconds after it appears. > Which line range is visible when you open the file and then when you hit > Ctrl+End? It seems to me the set of visible lines can have an impact on the > results. Line 93 to 126.
New Gerrit change created: https://git.eclipse.org/r/137375
This is most likely a symptom of bug 544708 . As a consequence, a sequence of Ctrl+End, (Ctrl+Home | Ctrl+End)* will only send a notification for the first Ctrl+End to ViewportListeners and ignores all other ones.
When codemining is on, other viewportListeners on the SourceViewer don't get notified neither.
The cause is that when codeminings are applied ` fTextWidget.getTopPixel()` returns an erroneous value (-10 in this case) when codeminings are on and prevent further execution of `TextViewer.updateViewportListeners()`.
New Gerrit change created: https://git.eclipse.org/r/137565
Tentative for 4.11.RC1
Gerrit change https://git.eclipse.org/r/137565 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=01afcdc0fb8782ec7a80728fedb1495e0212ed01
(In reply to Eclipse Genie from comment #31) > Gerrit change https://git.eclipse.org/r/137565 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=01afcdc0fb8782ec7a80728fedb1495e0212ed01 In master now, thanks for the fix Mickael.
Thanks for the review. I'd like to keep this open for some little time to get a change of adding some extra tests to codeminings by the way (see https://git.eclipse.org/r/137375 )
New Gerrit change created: https://git.eclipse.org/r/137652
Gerrit change https://git.eclipse.org/r/137652 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=33ab81d8513fb320fbe759c8b766299f584d5c3f
Why was the fix reverted?
(In reply to Mickael Istria from comment #33) > Thanks for the review. > I'd like to keep this open for some little time to get a change of adding > some extra tests to codeminings by the way (see > https://git.eclipse.org/r/137375 ) I've reverted that since I did not see Alex's +1/+2 on the change. I see he approved in this bug - sorry. However, I've added Lakshmi to review the change and she did not respond yet, so please wait for her review. Also, this change is at a very delicate position (used at many places by textual editors) and it's probably risky (just my gut feeling). I want at least Lakshmi's review here.
> I've reverted that since I did not see Alex's +1/+2 on the change. I see he approved in this bug - sorry. However, I've added Lakshmi to review the change and she did not respond yet, so please wait for her review. Ok, no big deal, and Lakshmi review is for sure welcome. In the meantime, let's defend this patch ;) > Also, this change is at a very delicate position (used at many places by > textual editors) and it's probably risky (just my gut feeling). setLineVerticalIndent() was introduced recently, in 4.10. So I doubt that beyond code minings, there are many editors that use it. The worst case scenario would be about performance: we reset the value so it needs to be recomputed later and if we reset it very often, it needs to be recomputed very often. But given that a similar line is invoked in resetCache in many cases, I don't think it has a noticeable effect.
(In reply to Mickael Istria from comment #38) > > I've reverted that since I did not see Alex's +1/+2 on the change. I see he approved in this bug - sorry. However, I've added Lakshmi to review the change and she did not respond yet, so please wait for her review. > > Ok, no big deal, and Lakshmi review is for sure welcome. > > In the meantime, let's defend this patch ;) > > > Also, this change is at a very delicate position (used at many places by > > textual editors) and it's probably risky (just my gut feeling). > > setLineVerticalIndent() was introduced recently, in 4.10. So I doubt that > beyond code minings, there are many editors that use it. > The worst case scenario would be about performance: we reset the value so it > needs to be recomputed later and if we reset it very often, it needs to be > recomputed very often. But given that a similar line is invoked in > resetCache in many cases, I don't think it has a noticeable effect. OK, that sounds less scary then. Performance might be an issue though as Code Mining is already not that fast.
New Gerrit change created: https://git.eclipse.org/r/137656
@Mickael, @Dani, Sorry for the delay in review (and reverts), I had to log-out a bit early today. I'm unable to reproduce the problem with Eclipse, but can reproduce the failure with the JUnit testcase and patch fixes the problem. I've posted my comments on the gerrit patch.
Gerrit change https://git.eclipse.org/r/137656 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=5f62a4a1adeb10f11945002f188bd16f4d3e2121
Thanks everyone!
Gerrit change https://git.eclipse.org/r/137375 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=dc3fc7c378f03dcd8404fd598641aa43a8ab086a
(In reply to Eclipse Genie from comment #44) > Gerrit change https://git.eclipse.org/r/137375 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/ > ?id=dc3fc7c378f03dcd8404fd598641aa43a8ab086a This caused 4 new fails on Mac: https://download.eclipse.org/eclipse/downloads/drops4/I20190227-1800/testresults/html/org.eclipse.jface.text.tests_ep411I-unit-mac64-java8_macosx.cocoa.x86_64_8.0.html
New Gerrit change created: https://git.eclipse.org/r/137772
(In reply to Andrey Loskutov from comment #45) > This caused 4 new fails on Mac: OK, can someone who has a Mac please give a try to (In reply to Eclipse Genie from comment #46) > New Gerrit change created: https://git.eclipse.org/r/137772
(In reply to Andrey Loskutov from comment #45) > (In reply to Eclipse Genie from comment #44) > > Gerrit change https://git.eclipse.org/r/137375 was merged to [master]. > > Commit: > > http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/ > > ?id=dc3fc7c378f03dcd8404fd598641aa43a8ab086a > > This caused 4 new fails on Mac: > > https://download.eclipse.org/eclipse/downloads/drops4/I20190227-1800/ > testresults/html/org.eclipse.jface.text.tests_ep411I-unit-mac64-java8_macosx. > cocoa.x86_64_8.0.html And one fail on Linux: https://download.eclipse.org/eclipse/downloads/drops4/I20190227-1800/testresults/html/org.eclipse.jface.text.tests_ep411I-unit-cen64-gtk3-java8_linux.gtk.x86_64_8.0.html
(In reply to Mickael Istria from comment #47) > (In reply to Andrey Loskutov from comment #45) > > This caused 4 new fails on Mac: > > OK, can someone who has a Mac please give a try to > > (In reply to Eclipse Genie from comment #46) > > New Gerrit change created: https://git.eclipse.org/r/137772 The patch fixes the test failures on Mac locally.
Gerrit change https://git.eclipse.org/r/137772 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=a63ada6c742b2f49237c81d2ef827e9b6c7141ec
Windows tests are also failed.
Ping! Tests still fail in I20190228-0300.
(In reply to Dani Megert from comment #52) > Ping! Tests still fail in I20190228-0300. I'm waiting for the current build to complete to have a look at the screenshot generated by the failures, but I'm really not able to reproduce any of those failures locally, and the Linux tests passed on Gerrit. So if anyone else can give a hand and at least run the tests locally and report whether they work or not on their machine, this would be pretty useful.
I believe we see only 3 tests succeeding on linux, and all four failing for Mac / Windows. Given that I would disable all 4 tests, close this bug and open a new one to fix tests.
The Unit tests are supposed to create screenshots on failure (they do with the Gerrit build) nearby the log location. I couldn't find those screenshots in https://ci.eclipse.org/releng/view/Automated%20tests/job/ep411I-unit-cen64-gtk3-java8/ws/ . Does anyone know where those could be? (In reply to Andrey Loskutov from comment #54) > I believe we see only 3 tests succeeding on linux, and all four failing for > Mac / Windows. Given that I would disable all 4 tests, close this bug and > open a new one to fix tests. As you wish. In any case, without any 3rd pary assistance, those tests will never be fixed anyway and regressions will happen.
New Gerrit change created: https://git.eclipse.org/r/137927
(In reply to Mickael Istria from comment #55) > (In reply to Andrey Loskutov from comment #54) > > I believe we see only 3 tests succeeding on linux, and all four failing for > > Mac / Windows. Given that I would disable all 4 tests, close this bug and > > open a new one to fix tests. > > As you wish. In any case, without any 3rd pary assistance, those tests will > never be fixed anyway and regressions will happen. Mickael, it does not make sense to create tests that fail from very beginning. Such tests will never catch any regression, because no one will ever notice that (look, they fail already on other platforms, so what?). Additionally, they cause a bad habit: https://en.wikipedia.org/wiki/Broken_windows_theory. If there are 10 tests failing, 4 more failing tests aren't that bad... (In reply to Eclipse Genie from comment #56) > New Gerrit change created: https://git.eclipse.org/r/137927 I believe I have a fix. Looks like display.post() did not work as expected for the new tests. At least on Windows they do not fail for me anymore.
Gerrit change https://git.eclipse.org/r/137927 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=8a10c51f5ccf762ed179f8295036f4541dc434be
New Gerrit change created: https://git.eclipse.org/r/137952
(In reply to Eclipse Genie from comment #59) > New Gerrit change created: https://git.eclipse.org/r/137952 This patch disables new tests on Mac. Previous one fixed them on Windows/Linux.
Gerrit change https://git.eclipse.org/r/137952 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=29cf1e467edacd40d6c06f84f5a193541db9a274
Thanks a lot Andrey! Do you think Display.post(...) is buggy here?
(In reply to Mickael Istria from comment #62) > Thanks a lot Andrey! > Do you think Display.post(...) is buggy here? I think either our Linux test environment is "different" (like this SWT test fail [1] saying "Display#post failed, probably because screen is not rendered (bug 407862)"), or display.post() *behavior*, triggered from inside the test, is not same as real keyboard input. At least on Windows on my notebook with "usual" environment I could see that the post() is not triggering same chain of events as real user. Unfortunately I have no Mac, so no idea what is going wrong there. [1] https://download.eclipse.org/eclipse/downloads/drops4/I20190303-0600/testresults/html/org.eclipse.swt.tests_ep411I-unit-win32-java8_win32.win32.x86_64_8.0.html I'm resolving this bug now. Mickael, could you please create a bug to investigate & re-enable Mac tests?
