Community
Participate
Working Groups
Switch the Project Explorer to show working sets as top level elements. (Create a few Java working sets containing projects before this). Notice that the labels for projects are missing, but the decoration (e.g. CVS information) is still there. Attaching screenshot.
Created attachment 128487 [details] screenshot
This used to work in M4.
Looks bad, should get this into M6, I will fix it tonight.
(In reply to comment #3) > Looks bad, should get this into M6, I will fix it tonight. > It got too late and I did not get to this, I will look at it in the morning and try to have something for the 1pm build.
Created attachment 128582 [details] Proposed patch Ugh... More learning about the CN. It turns out that the (undocumented CNF) convention is that returning an empty string in an IStyleText label provider means that we want to bypass that label provider and look further. And when I fixed bug 262833 I broke this. Furthermore, the idea of recording the navigator content descriptor that contributed the extension and relying exclusively on that is not a good idea in this case. The JavaProject element here was contributed by the Workingset NCE, which of course has nothing to do with Java projects. In the M4 implementation it would still remember the workingset NCE, it would ask it for a label, and the workingset NCE would return "", so it would go on to the next one which was the Java NCE and get the correct label. With the previous fix to bug 262833, I broke the capability of the NCE returning a "" that means "I'm not interested, search for the next" (it took the "" and used it as a label). Also, my recent code assumed that if it found an NCE (in the cache), then it would for sure use that, but here is a situation where that is not desirable. So the fix for M6 is the most conservative I can come up with: back out the change for bug 262833, and change the behaviour of the finding a label provider to behave as it did for M4. I think this will provide the most compatibility, however, I'm really uncomfortable that we cache the NCE for objects that are really not related to the NCE that provided them (that is we are associating a JavaELement object with the workingsets NCE) -- I don't have a good idea about how to address this now, but it's not something that need to be done for M6. Boris, can you review my patch?
> And when I fixed bug 262833 I broke this. Correction bug 262883
Created attachment 128590 [details] theoretical patch For purposes of understanding your patch, in which way would this slightly changed patch be wrong? (I am worried about the performance implications.)
(In reply to comment #7) > Created an attachment (id=128590) [details] > theoretical patch > > For purposes of understanding your patch, in which way would this slightly > changed patch be wrong? > > (I am worried about the performance implications.) > I think it's OK, though I don't like it because it's more complex. It does however point out yet another problem with this whole mess. You could end up with a different NCE providing the text than the image. I would not worry about the performance implication of this, the proposed patch is exactly what was done in M4 and previous releases, so it will not be a regression. Since my proposed patch is close to the original implementation, I would prefer that rather than introduce the additional complexity. (But I know your real reason for proposing this, you just want to get back at me for the mess I made of StructuredViewer ;)
I have tried to apply the "theoretical patch". It does not help for any of bug 268384, bug 268385 or bug 268386. It even makes the things worse - other label providers in WTP break. I am using Platform build I20090311-1800 and applied the patch on top of it.
Hmm... If I just sync org.eclipse.ui.navigator to HEAD and my target platform points to I20090311-1800, then bug 268386 is resolved. I still have additional label providers broken, so this is not caused by the "theoretical patch". I also tried applying "Proposed patch", but it does not change anything. I hope this information helps.
(In reply to comment #10) > Hmm... If I just sync org.eclipse.ui.navigator to HEAD and my target platform > points to I20090311-1800, then bug 268386 is resolved. > > I still have additional label providers broken, so this is not caused by the > "theoretical patch". > > I also tried applying "Proposed patch", but it does not change anything. > > I hope this information helps. > Odd, I don't think HEAD is any different than the 0311 build for the navigator. I'm not sure what to do about this. These sort of problems sound like some of the issues that we had with M5 with the CNF, but they were addressed in M6.
(In reply to comment #9) > I have tried to apply the "theoretical patch". It does not help for any of bug > 268384, bug 268385 or bug 268386. It even makes the things worse - other label > providers in WTP break. Kaloyan, could you please give us detailed steps for how we can test this here? What should we install into our Eclipse, which projects should we check out from CVS, and what are the steps to reproduce the problems? The more information you can give us, and the sooner you can do this, the better M6 will be for you. Note that the original problem I reported in this bug are resolved by both "Proposed patch" and "theoretical patch".
Created attachment 128618 [details] My Workspace All of the CN test cases should pass with this except one, and that's the test we are trying to make work.
Created attachment 128624 [details] patch for 1800 build releasing this for the 1800 build
Francis, I should be back online at 8 pm my time. In addition to the two changes we talked about, I changed one more line relative to your "Proposed patch", we should talk about this and test some more: Set theDescriptorInstances = findContentExtensionsByTriggerPoint(anElement, true, CONSIDER_OVERRIDES);
Created attachment 128635 [details] Latest patch from HEAD This makes the tests pass and should most closely match the previous behaviour.
Created attachment 128643 [details] Current experimental patch
Created attachment 128646 [details] Hopefully this is it
Created attachment 128648 [details] With jface this time
Released to HEAD, I20090313-0100, 35M6
*** Bug 268353 has been marked as a duplicate of this bug. ***
*** Bug 268384 has been marked as a duplicate of this bug. ***
*** Bug 268386 has been marked as a duplicate of this bug. ***
Just curious, if it's easy for you to say ... are there any compile time dependencies with the fixes? I'm trying to decide if we should re-build the weekly WTP build, or just finish our weekly testing with latest I-build?
(In reply to comment #24) > Just curious, if it's easy for you to say ... are there any compile time > dependencies with the fixes? I'm trying to decide if we should re-build the > weekly WTP build, or just finish our weekly testing with latest I-build? > You can use the I-build (you don't need to recompile). The whole point of this was to make sure the CNF/platform worked in a binary compatible way.
Hey, I synched org.eclipse.jface and org.eclipse.ui.navigator to HEAD and everything is just perfect. Thank you for this quick resolution!
*** Bug 265605 has been marked as a duplicate of this bug. ***
*** Bug 263921 has been marked as a duplicate of this bug. ***
Verified on Windows XP using I20090313-0100. Verification included three of the WTP use cases.
Boris can you give me a link to the I20090313-0100 build? Following the download page, I reached the following binary: http://download.eclipse.org/eclipse/downloads/drops/I20090313-0100/eclipse-SDK-I20090313-0100-win32.zip But it is just 69 KB in size and does not seem to work...
(In reply to comment #30) > Boris can you give me a link to the I20090313-0100 build? > > Following the download page, I reached the following binary: > http://download.eclipse.org/eclipse/downloads/drops/I20090313-0100/eclipse-SDK-I20090313-0100-win32.zip > > But it is just 69 KB in size and does not seem to work... > Sounds like the build has not replicated yet. Can you wait an hour and then try again?
