Community
Participate
Working Groups
Created attachment 83453 [details] Patch with fix. The modules view itself is already based on the flexible hierarchy viewer and it can be populated with contexts from other debug models (such as DSF). However the detail pane is still fixed to use ICModule and ICElement interfaces. I created a patch which modifies the modules view to use the detail pane extension point mechanism from platform debug. Unfortunately right now the APIs to use this extension point are internal to platform debug (see bug 210466). I recommend to wait before applying this patch to see if platform will open up that API. If it does not, I would recommend duplicating this mechanism in CDT debug plugin for use in the Modules view.
Using internal APIs would be par for the course with the Modules view...
As far as I know CDT debug, (and DSF as well) only uses _provisional_ platform debug APIs. The provisional APIs are in internal packages because they are subject to change in point releases, but they are intended to become public API at some point. The detail pane API that this patch uses is an internal API which as far as I know was not intended to become public at any point. But maybe with this use case we can change that.
Understand. I was just exhibiting my frustration at how often Platform changes have been causing compile errors in the Modules View ;)
We have duplicated some internal platform classes in the past (not for the Modules view) and it requires a constant attention. Someone has to follow the changes and bug fixes in the original implementation which is simply imposible because we don't have people assigned to this project. As a result there are lots of old code in CDT, the platform has moved on since but we still using the old classes. On the other hand, using internal classes is a pain as Doug mentioned, but it keeps as alert and forces to follow the latest platform changes. At the same time the platform people are not very keen to make the UI components public (which is understandable). It's been an issue for a long time. See for instance https://bugs.eclipse.org/bugs/show_bug.cgi?id=187500. I'll take a look at the patch.
All of the org.eclipse.debug.internal.ui.views.variables.details classes look like good candidates to be a public API, but I suspect Darin has different opinion. I would suggest to use those classes directly instead of copying it to CDT -duplicating DetailPaneManager doesn't seem like a good idea. Please, let me know which approach is preferable.
(In reply to comment #5) > All of the org.eclipse.debug.internal.ui.views.variables.details classes look > like good candidates to be a public API, but I suspect Darin has different > opinion. > I would suggest to use those classes directly instead of copying it to CDT > -duplicating DetailPaneManager doesn't seem like a good idea. > Please, let me know which approach is preferable. > I agree that keeping up with changes in duplicated code is more error prone than just referencing internal APIs. But it does carry the risk of the build breaking at random times. I can volunteer to submit patches to keep up with internal platform API changes, but I think this is a decision the Doug should make as it is a headache whenever the build breaks.
(In reply to comment #6) > I agree that keeping up with changes in duplicated code is more error prone > than just referencing internal APIs. But it does carry the risk of the build > breaking at random times. It seems that most of the people use the milestone builds so we need to apply current changes when we switch form one milestone to another. > I can volunteer to submit patches to keep up with internal platform API > changes, but I think this is a decision the Doug should make as it is a > headache whenever the build breaks. I have been doing it since I changed the Modules view and it is very unpleasant task. You have to do it in a very short time (usually during a weekend) and you always get criticized for API breakage. If you can wait with this patch we can discuss it at the next CDT call. In general, IMHO the Modules view belongs to the Platform. The maintanence is easy because its design is the same as for the Variables view. As far as I know IBM ships their own Modules view with very similar functionality. We (Alain and I) tried to talk to them about it but it didn't work. Maybe we should try it again.
(In reply to comment #7) > If you can wait with this patch we can discuss it at the next CDT call. Certianly, I think that should be next week anyway. > In general, IMHO the Modules view belongs to the Platform. The maintanence is > easy because its design is the same as for the Variables view. As far as I know > IBM ships their own Modules view with very similar functionality. We (Alain and > I) tried to talk to them about it but it didn't work. Maybe we should try it > again. I filed bug 211158 with Platform/Debug to get a comment on this request.
(This is pending platform acceptance of patch in bug 211158). The following patch removes the CDT modules view, with use of the Modules view ported to debug platform. The patch is rather extensive as it builds on the detail pane patch submitted previously and it adds more extensive use of flexible hierarchy view providers. The irony of this change is that removing the Modules view from CDT removes dependencies on org.eclipse.debug.ui internal variables view classes. However, because of a limitation in the flexible hierarchy interfaces, it adds a dependency on some base classes of flexible hierarchy providers, which are also internal to platform (and not a provisional API). IMO, this second dependency is lesser of the two evils, as the flexible hierarchy APIs design will need to solve the general problem of adapter hierarchies, where as the variables view is unlikely to ever become public API.
Created attachment 88215 [details] Patch with use of the pending platform Modules view.
(In reply to comment #9) > (This is pending platform acceptance of patch in bug 211158). > > The following patch removes the CDT modules view, with use of the Modules view > ported to debug platform. The patch is rather extensive as it builds on the > detail pane patch submitted previously and it adds more extensive use of > flexible hierarchy view providers. > > The irony of this change is that removing the Modules view from CDT removes > dependencies on org.eclipse.debug.ui internal variables view classes. However, > because of a limitation in the flexible hierarchy interfaces, it adds a > dependency on some base classes of flexible hierarchy providers, which are also > internal to platform (and not a provisional API). IMO, this second dependency > is lesser of the two evils, as the flexible hierarchy APIs design will need to > solve the general problem of adapter hierarchies, where as the variables view > is unlikely to ever become public API. > Man, that is ironic and unfortunate. Is there a proposal on the table to address the flexible hierarchy issues or does it need more investigation/work. Just wondering when we can expect to finally get rid of our use of internals.
(In reply to comment #11) > Man, that is ironic and unfortunate. Is there a proposal on the table to > address the flexible hierarchy issues or does it need more investigation/work. > Just wondering when we can expect to finally get rid of our use of internals. Right now all the flexible hierarchy interfaces are still a provisional API. And I think the biggest issues to making them final has been maturity and resources. This is also a big issue for DSF and for any debugger which uses these interfaces. As 3.4 is third point release that these APIs are in, I expect that they will finally be made official in 4.0. Like I said though, even if these APIs become official, some of the classes that the Modules view integration requires are still plugin private: DefaultModelProxyFactory, DebugTargetContentProvider, etc. These classes are something of a problem for platform because due to their design, as a public API they could be difficult to evolve and maintain. However, any debugger trying to extend standard debug model and use flexible hierarchy needs to either extend or copy these default implementation classes. I think this question will also need to be answered when the flexible hierarchy APIs go official. Personally, I think there's another way to address this problem: to use UI part of DSF to implement the default flexible hierarchy providers for the standard debug model. However, DSF uses wrappers and this would certainly break the backward compatiblity with the current APIs. Still, the DSF-based default providers should be easier to maintain as an API so it should be easier to make them public. I've been tempted to try to implement this over the last year, but Wind River doesn't really have a business reason for this functionality. Perhaps if anyone else has this problem, they could investigate this solution with my help.
(In reply to comment #12) > Personally, I think there's another way to address this problem: to use UI part > of DSF to implement the default flexible hierarchy providers for the standard > debug model. However, DSF uses wrappers and this would certainly break the > backward compatiblity with the current APIs. Still, the DSF-based default > providers should be easier to maintain as an API so it should be easier to make > them public. I've been tempted to try to implement this over the last year, > but Wind River doesn't really have a business reason for this functionality. > Perhaps if anyone else has this problem, they could investigate this solution > with my help. > So is the basic idea that DSF would, in effect, duplicate the classes that are currently internal to the platform (DefaultModelProxyFactory, DebugTargetContentProvider, etc), with the key difference that they will be public in DSF? If I have that right, then the advantage this would offer it that custom view code that extends the standard model could (a) avoid the risk of using platform internal classes: that of being broken by a a future platform release (b) avoid the headaches of doing a cut-n-run of those platform internal classes, effectively relying on DSF to do that maintenance work (picking up fixes and improvements from the platform implementations) (c) rely on DSF to evolve forward with new standard model adapter implementations that exist side by side with existing ones, thus providing (a) but not forcing new feature development to rely on stagnant implementations when new-and-improved ones are possible Is my understanding correct? John
(In reply to comment #13) > ... > Is my understanding correct? Yes, I would also add: (d) Picking up features that are easier to implement with DSF-based adapters than with the current standard model adapters. Currently selectable update modes is the only feature that stands out, but given DSF adapter design it should also be possible to implement: filtering, sorting, tracking change history, and showing diffs between elements in a view. But the important assumption here is that DSF-based adapters would actually be easier to maintain. Unfortunately, it's rather hard to make such a claim without actually doing it :-)
(In reply to comment #12) > Personally, I think there's another way to address this problem: to use UI part > of DSF to implement the default flexible hierarchy providers for the standard > debug model. However, DSF uses wrappers and this would certainly break the > backward compatiblity with the current APIs. I am not familiar enough with DSF to understand this. Can you please explain what exactly would be broken if this approach is implemented.
(In reply to comment #15) > I am not familiar enough with DSF to understand this. Can you please explain > what exactly would be broken if this approach is implemented. DSF uses wrappers for model elements to populate the views. This means that any action or other selection listener (e.g. objectContribution extensions) which expects an object of a certain type in the view, will not work anymore. Of course, it's not too big a deal to port the actions and listeners so that they use IAdaptable or the wrapper interfaces to gain access at the actual element, however it is a big deal when strict backward compatibility is required.
(In reply to comment #16) > (In reply to comment #15) > > I am not familiar enough with DSF to understand this. Can you please explain > > what exactly would be broken if this approach is implemented. > DSF uses wrappers for model elements to populate the views. This means that > any action or other selection listener (e.g. objectContribution extensions) > which expects an object of a certain type in the view, will not work anymore. Does it mean that any model should implement DSF wrappers for it's elements to be "visible" in the view?
(In reply to comment #17) > Does it mean that any model should implement DSF wrappers for it's elements to > be "visible" in the view? This is a very general statement. To clarify a bit: DSF includes a set of UI-level APIs called "view model" which build on flexible hierarchy providers. It is possible to implement these APIs to populate debugger views with data from the standard debug model interfaces. However, part of the DSF's view model API design is the requirement that the model elements are not stored in the view directly, but rather stored inside wrapper objects. The wrapper object interface is part of the view model API.
(In reply to comment #18) Do you use the "objectContribution" extension point with DSF wrappers?
(In reply to comment #19) > (In reply to comment #18) > Do you use the "objectContribution" extension point with DSF wrappers? > No. If need be will use commands with appropriate visibility expression to achieve the same result. BTW. It's probably more appropriate to take this discussion to the mailing list. If you have more questions please direct them there.
bug 211158 was applied in Debug Platform! This means that when switching to M6 milestone, this bug can be applied and the CDT-specific modules view can be removed.
I'm going to miss the griping about the occasional build break when moving to a new platform milestone. Can we not introduce a dependency on Darin's provisional APIs somewhere else in CDT ;-)
I don't know if you read comment #9, but we are actually introducing new dependencies on Darin's provisional APIs. We are replacing dependencies on internal classes though (not even provisional stuff), so I hope it's a net positive.
I was part of that discussion :-) I was assuming we'd go the route you recommended of using DSF to shield us.
Created attachment 93537 [details] Patch with updated fix. I found a small problem with restoring expanded state and selection when testing the new Platform Modules view today. The updated patch fixes the problem.
Applied. Thanks Pawel.
