Community
Participate
Working Groups
Build Identifier: 201009241320 Navigating through the code via Ctrl+LeftMouseButtonClick is not working anymore since upgrade to CDT 7.0.1. Navigation via F3 still works fine. That means most of the time I move the mouse over an item and press Ctrl the item is not underlined to indicate that I can click it. Sometimes it is possible that an item is underlined properly and sometimes it might happen, that an item becomes underlined after some seconds. Because navigation via F3 is working properly it is not an indexer issue of my project. The problem occures in all my elicpse projects. So I think Ctrl+Click navigation is broken in some way. Reproducible: Sometimes Steps to Reproduce: 1. Press Ctrl and move mouse over an include file 2. If include include file is underlined and you can click it, then click it 3. Repeat step 1.
Probably due to bug 324232.
The UI blocking was less noticeable to me. Maybe the fix in bug 324232 should be rolled back and the bug reopened?
The blocking behavior due to bug 324232 was a more serious problem than the delay in showing hyperlinks. I'm open to ideas how to solve the problem in a less intrusive way, but against rolling back the fix for bug 324232 until we have a better solution.
I've noticed what appears to be the same problem since upgrading from CDT 6.x to 7.0.1: * Ctrl-Click seems to always not work when a source file is first opened in the editor or when switching tabs from one already opened file to another. * F3 works just fine. * If I mouse over an identifier and wait for the hover tip showing where that identifier is defined to appear, then holding down Ctrl properly hyperlinks identifiers, and Ctrl-Click works fine until the next time I open another file or switch tabs to an already opened file again.
Hi, I would like to emphasize the severity of this bug. In my opinion, this is very basic functionality used by many developers for code navigation. I think it should be solved as quickly as possible and maybe some patch could be added to already released 7.0.1 (not sure if possible). Unfortunately our development team cannot adopt 7.0.1 CDT as long as this feature is broken. Thanks, Yevgeny
(In reply to comment #5) This issue presents a tough dilemma. In order for Ctrl+Click to work, CElementHyperlinkDetector has to determine if there is navigable symbol at the current cursor location. In order to do that CElementHyperlinkDetector needs an AST. AST takes considerable time to build. The behavior before 7.0.1 was to wait for the AST to be built whenever Ctrl key is pressed. This behavior was problematic since the Ctrl key doesn't uniquely identify user intent and is used as part of various hot keys, most often with Ctrl+C and Ctrl+V. Waiting for the AST in CElementHyperlinkDetector resulted in freezing the UI when user was trying to copy and paste or was doing other actions involving Ctrl key. Based on the principle that freezing the UI is the worst sin, the behavior was changed in 7.0.1. CElementHyperlinkDetector checks for availability of the AST, but doesn't wait for it to be built if it's not yet available. As a result, there is a time gap between opening an editor and before the hyperlinks can be shown. This time gap existed before 7.0.1, but the UI would simply freeze for the duration of the gap and the hyperlink would be eventually shown. I can think of the following imperfect solutions: 1. Introduce a preference allowing user to choose between the wait and no-wait behaviors. Doesn't really solve the problem, but allows user to make her own choice between the two evils. 2. Change CElementHyperlinkDetector to always display a link when the AST is not ready yet. Once user clicks on the link, it is checked using the AST. This may result in showing links in weird places, for example on C/C++ reserved words. False links won't be clickable. 2a. In addition to 2 we can do some shallow parsing to eliminate some of the false links. If you have other ideas how to solve this problem, please express them here. Ideas accompanied by patches are even more welcome.
> but doesn't wait for it to be built if it's not yet available. As a result, > there is a time gap between opening an editor and before the hyperlinks can be > shown. This time gap existed before 7.0.1, but the UI would simply freeze for > the duration of the gap and the hyperlink would be eventually shown. In 7.0.1 it may last 10 or event more than 10 seconds for us before the links are clickable while F3 navigation works immediatelly. In CDT 7.0 I never noticed UI freezing when clickng Ctrl key. If I navigate through the code I expect a very fast navigation experience because that is an essential thing when writing C++ code. I may have to search through a number of header files (i.e. when working with Qt), declaration and definitions. This happens always and any time when writing code. So reading through existing code and navigating through existing code often takes more time than writing code. So in my opinion this is a very essential and basic feature for working with an IDE that is used all the time by the user. Therefore the speed of this feature needs to be very high. So working with CDT 7.0 was much smoother for us and we did not notice (or remember) any UI freezing while clicking Ctrl key in 7.0 - so it must have happened very seldom on.
(In reply to comment #6) > As a result, > there is a time gap between opening an editor and before the hyperlinks can be > shown. This time gap existed before 7.0.1, but the UI would simply freeze for > the duration of the gap and the hyperlink would be eventually shown. What I'm experiencing is that now the "time gap" has to be triggered by something first. Like Josh wrote, a hover on a identifier can trigger it or clicking somewhere (triggering mark occurrences) or opening call hierarchy and probably anything that asks for the AST. But if you don't do any of that, the hyperlinks will never show up. (In reply to comment #6) > If you have other ideas how to solve this problem, please express them here. > Ideas accompanied by patches are even more welcome. Is there a way this could be done asynchronously?
(In reply to comment #8) > What I'm experiencing is that now the "time gap" has to be triggered by > something first. Like Josh wrote, a hover on a identifier can trigger it or > clicking somewhere (triggering mark occurrences) or opening call hierarchy and > probably anything that asks for the AST. But if you don't do any of that, the > hyperlinks will never show up. Interesting observation. We should probably trigger building of AST as soon as editor receives focus not relying on the reconciler. > > (In reply to comment #6) > > If you have other ideas how to solve this problem, please express them here. > > Ideas accompanied by patches are even more welcome. > > Is there a way this could be done asynchronously? IHyperlinkDetector.detectHyperlinks method is synchronous and runs on the UI thread. This is defined by the Platform.
Created attachment 180109 [details] Build AST in another thread when not available This patch will build the AST in another thread if it's not available yet. The problem with this approach is that the user still needs to do a mouse move to get the hyperlink after the job is done. (In reply to comment #9) > (In reply to comment #8) > > What I'm experiencing is that now the "time gap" has to be triggered by > > something first. Like Josh wrote, a hover on a identifier can trigger it or > > clicking somewhere (triggering mark occurrences) or opening call hierarchy and > > probably anything that asks for the AST. But if you don't do any of that, the > > hyperlinks will never show up. > > Interesting observation. We should probably trigger building of AST as soon as > editor receives focus not relying on the reconciler. I think that would make sense. It would reduce the initial delay too. > > (In reply to comment #6) > IHyperlinkDetector.detectHyperlinks method is synchronous and runs on the UI > thread. This is defined by the Platform. Yes, I guess I was asking, would it be possible to make it asynchronous even if it requires changing the Platform. But maybe that's overkill.
Created attachment 180125 [details] Detect hyperlinks without AST when AST is not available I've realized that hyperlinks can be detected with reasonably good fidelity even without an AST. The attached patch falls back on non-AST algorithm when the AST is not ready yet. I'm going to submit this to HEAD now and to 7.0.x once people get chance to play with it.
Created attachment 180127 [details] Added a test for hyperlink detection without AST
*** cdt cvs genie on behalf of sprigogin *** Bug 326418 - Ctrl+Click navigation is broken. [*] HyperlinkTest.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/HyperlinkTest.java?root=Tools_Project&r1=1.11&r2=1.12 [*] CElementHyperlinkDetector.java 1.20 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/CElementHyperlinkDetector.java?root=Tools_Project&r1=1.19&r2=1.20
(In reply to comment #11) > the AST is not ready yet. I'm going to submit this to HEAD now and to 7.0.x > once people get chance to play with it. I've tried the patch, it works reasonable well. I noticed a couple of minor problems: - overloaded operators - vector::end(), map::end()
Created attachment 180224 [details] Fixed an issue with treating "end" as a language keyword.
*** cdt cvs genie on behalf of sprigogin *** Bug 326418 - fixed false positives in language keyword check. [*] CElementHyperlinkDetector.java 1.21 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/CElementHyperlinkDetector.java?root=Tools_Project&r1=1.20&r2=1.21
*** cdt cvs genie on behalf of sprigogin *** Bug 326418 - Ctrl+Click navigation is broken [*] CElementHyperlinkDetector.java 1.18.6.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/CElementHyperlinkDetector.java?root=Tools_Project&r1=1.18.6.1&r2=1.18.6.2 [*] HyperlinkTest.java 1.10.2.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/HyperlinkTest.java?root=Tools_Project&r1=1.10.2.1&r2=1.10.2.2
Fixed in HEAD and cdt_7_0 > 20101006.
Hi, In comment 14, issue with overloaded operators was mentioned. Was this issue fixed as well? Thank you for handling this quickly. Is it possible to provide some kind of patch for 7.0.1 CDT? Thanks, Yevgeny
(In reply to comment #19) > Hi, > > In comment 14, issue with overloaded operators was mentioned. Was this issue > fixed as well? No, this is a limitation of the no-AST approach. You can create a separate bug for it if you like. > Is it possible to provide some kind of patch for 7.0.1 CDT? You should be able to get it from the next 7.0.2 build when it becomes available.
(In reply to comment #20) > (In reply to comment #19) > > Hi, > > > > In comment 14, issue with overloaded operators was mentioned. Was this issue > > fixed as well? > > No, this is a limitation of the no-AST approach. You can create a separate bug > for it if you like. I think it should request the AST so that the AST approach becomes available at some point. Also, other hyperlinks contributed might need the AST, for example bug 319480.
(In reply to comment #21) > I think it should request the AST so that the AST approach becomes available at > some point. Also, other hyperlinks contributed might need the AST, for example > bug 319480. I agree that AST should start building as soon as an editor becomes active. We shouldn't wait for Ctrl to be pressed to trigger AST build.
(In reply to comment #20) > (In reply to comment #19) > > Hi, > > > > In comment 14, issue with overloaded operators was mentioned. Was this issue > > fixed as well? > No, this is a limitation of the no-AST approach. You can create a separate bug > for it if you like. > > Is it possible to provide some kind of patch for 7.0.1 CDT? > You should be able to get it from the next 7.0.2 build when it becomes > available. Hi, This bug was reported for regression in Ctrl+LeftMouseButtonClick functionality in 7.0.1 in compare to 7.0.0. Solution was provided, which does not fully solve this issue (overloaded operators). I think this bug should be closed once the functionality is working as in 7.0.0. If you insist I will open a new bug, but I don't understand why is it needed? Thanks, Yevgeny
Created bug 338004 for overloaded operators.