Bug 213069 - Extend PresentationContext to save and restore properties.
Summary: Extend PresentationContext to save and restore properties.
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.4   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 3.4 M5   Edit
Assignee: Platform-Debug-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-12-14 17:33 EST by Pawel Piech CLA
Modified: 2008-01-29 11:46 EST (History)
1 user (show)

See Also:


Attachments
Patch with the enhancment. (10.23 KB, patch)
2007-12-14 17:33 EST, Pawel Piech CLA
no flags Details | Diff
Updated patch with enhancment. (10.25 KB, patch)
2007-12-14 19:36 EST, Pawel Piech CLA
no flags Details | Diff
updated patch (12.33 KB, patch)
2008-01-29 11:39 EST, Darin Wright CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pawel Piech CLA 2007-12-14 17:33:39 EST
Created attachment 85308 [details]
Patch with the enhancment.

The IPresentationContext is used by the variables views and others to hold certain viewer properties, such as: selected columns, show logical structures, selected number format, selected update mode, etc.  

However currently, these properties are not automatically seriazlized by the view, so the actions which control these properties must create their own mechanism to do so.  There is currently a mechanism in the variables views which preserves the state of toggle actions in the tool bar, but there is nothing equivalent for any other actions. 

The included patch adds methods to save and restore the properties values in the default implementation of IPresentationContext.  It also adds calls to these methods in the variables views as well as tests for these methods.
Comment 1 Pawel Piech CLA 2007-12-14 19:36:54 EST
Created attachment 85316 [details]
Updated patch with enhancment.

Fixed an NPE in the previous patch.
Comment 2 Darin Wright CLA 2008-01-03 16:55:08 EST
This looks reasonable. The only issue is that anyone who makes a "tree model viewer" (i.e. viewer with a presentation context), must explicitly persist/restore the presentation context settings. So, we need to add code to the LaunchView and MemoryTreeViewer to do the same as the VariablesView - i.e. call init and save on the presentation context, with the appropriate memento.

I'm not sure where we should document this persistence, as it is really up to the view and presentation context to decide whether persistence will be supported. If we document the persistence, we also need to document what types of objects get persisted (i.e. only Strings, Integers, ... etc.). It feels like the views that support persisted properties need to document the behavior (but there's not really a good place to do this).
Comment 3 Pawel Piech CLA 2008-01-03 17:36:22 EST
(In reply to comment #2)
That's a very good point.  I did not add the persistence methods to the IPresentationContext interface, because I thought it was really an implementation detail of the view that was creating the context.  

The only public references to the views are their ID definitions in the IDebugUIConstants class, and this is not a logical place to look for this type of information.  Perhaps there needs to be a package javadoc page for each view which simply states that the views support such and such flexible hierarchy APIs and they persist properties set to the presentation context.  If you need help with this, I could put something like that together as part of this bug.

But probably the most appropriate place for such documentation would be in the Programmer's Guide in the Platform Plugin Developer's Guide.  It seems like this documentation would need to be updated to include information about flexible hierarchy, including the presentation contexts.  Then again flexible hierarchy are still internal APIs so in that sense it's too soon for that.
Comment 4 Darin Wright CLA 2008-01-29 11:39:52 EST
Created attachment 88152 [details]
updated patch

Updated the patch. I think this simplifies the logic. Saving/restoring is performed when calling TreeModelViewer.saveState(IMemento)/initStat(...). This way, if a viewer's state is saved/restored, the presentation context is also saved/restored at the same time.

The upside is that there are no changes to the Variable's view, and would be no changes to other views that already call save/restore.
Comment 5 Darin Wright CLA 2008-01-29 11:40:22 EST
Applied patch.
Comment 6 Darin Wright CLA 2008-01-29 11:40:47 EST
Verified.
Comment 7 Pawel Piech CLA 2008-01-29 11:46:19 EST
(In reply to comment #4)
> Updated the patch. I think this simplifies the logic. Saving/restoring is
> performed when calling TreeModelViewer.saveState(IMemento)/initStat(...). This
> way, if a viewer's state is saved/restored, the presentation context is also
> saved/restored at the same time.

Sounds like a fine idea :-)  Many thanks again for applying the patch.