Community
Participate
Working Groups
Created attachment 151903 [details] Proposed fix Sometimes, the JDT debug hover does not show during a debug session. Instead, it is the Source hover that is shown. To reproduce: 1- start eclipse without activating the org.eclipse.jdt.debug plugin. To do this, start in the Java perspective, without any debugging views open. 2- Select a java editor and hover over a variable. The Source hover will trigger. 3- launch a java debug session 4- during that session hover over a variable in the editor. You will again see the Source hover instead of the Debug hover. The way that BestMatchHover decides which hover to use is by going through each hover sequentially to find the first one that will return an output. This implies that the order in which the hovers are queried is very important. The bug is caused by the fact that if a plugin is not activated, its hover is not added to the hover list of BestMatchHover right away, but will be added later, once the plugin is activated. And when it is finally added it is put at the end of the list, losing its proper position. This is because on the step 2, the debug plugin was not active so did not create the debug hover. On step 4, the debug hover is finally created, but it is added to the end of the hover list, after the Source hover. The attached patch proposes a fix. The patch keeps the API the same even though the class was internal. I also tried to keep performance as least impacted as possible, although there is an impact; however, the list of hovers is so small that it should not matter. Note that I found this problem because we had an exact copy of BestMatchHover in the CDT, and that is where I saw and fixed the problem (bug 294812)
Good catch! I fixed it differently by allowing 'null' inside fInstantiatedTextHovers and fTextHoverSpecifications.
(In reply to comment #1) > Good catch! > > I fixed it differently by allowing 'null' inside fInstantiatedTextHovers and > fTextHoverSpecifications. Thanks for getting to this so fast! I had used the annoying global variable fNullEntryCount in an attempt to keep using addTextHover() as I thought that maybe someone could be overriding it. But if that is not important, I agree it's better not to use the global var. One tiny comment, in checkTextHovers(), the for loop creates a new ArrayList. I assume this was to allow removing elements from fTextHoverSpecifications, without screwing up the iterator. Since you no longer remove such elements, I believe you don't need this new ArrayList, which just wastes cycles to create. (In fact, I'd replace the whole use of lists with arrays in this file; but it's probably not worth the effort).
Thanks for the tip. BTW: why did you change all the other attributes in this bug?
(In reply to comment #3) > Thanks for the tip. > > BTW: why did you change all the other attributes in this bug? That's weird, I didn't do that consciously. Maybe I hadn't refreshed the bug page properly and my browser still had the old settings... Sorry about that.
