Bugzilla – Full Text Bug Listing
| Summary: | [Presentations] Allow for unanticipated parameters for part appearance and behavior | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Chris Gross <schtoo> | ||||||||||
| Component: | UI | Assignee: | Paul Webster <pwebster> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P3 | CC: | emoffatt, Mike_Wilson, n.a.edgar | ||||||||||
| Version: | 3.1 | Keywords: | helpwanted | ||||||||||
| Target Milestone: | 3.3 M5 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows XP | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Chris Gross
I would like to add a use case for this. This is primarily needed for RCP applications. Consider that our security mechanism may prevent people from having full access to different parts of the application. Sometimes they may be able to see but not modify the objects within a certain editor. I'd like the ability to just tell the presentation that this is a read-only part and present that somehow in the UI. Perhaps this is a decorator on the tab, perhaps just a different color. We are actually doing this right now. But because there is no way to pass parameters to the presentation, we are cramming everything in the content description as a delimited string. Possible implementation: The initial bug I entered for this was bug 86357 that asked for access to the getAdapter method of the underlying part. This is problematic because the part is not instantiated/activated until after the presentation is established. But couldn't we just tie it together using the property change listener just like is done with the other properties and toolbar and such. IPresentablePart could have a getAdapter method but the method would return null or perhaps throw an exception until after the part was activated. Once the part is activated a property change event would be thrown with IPresentablePart.PROP_ADAPTER (or whatever). Thereby alerting the presentation that its safe to call getAdapter. There are currently no plans to work on this feature. PW Investigating. PW What kind of information should be propogated across?
A possible option is to define
interface IWorkbenchPartProperty {
/**
* Return a user defined property or null if it doesn't exist.
*/
public String getProperty(String key);
/**
* set a user defined property. Fires a property change event
* with PROP_USER. A value of null clears the property.
*/
public void setProperty(String key, String value);
}
and implement it on WorkbenchPart.
Then add
public String getProperty(String key)
to IWorkbenchPartReference and IPresentablePart. It would return null if the part doesn't exist yet and the reference has not been persisted from a previous session (assuming that is possible).
PW
Ideally I'd like the properties to be Objects but I Strings will do. I can see the persistance would be messy otherwise. So, yeah what you have would work for me. I'm pretty sure that I can persist (String, String) properties from the IWorkbenchPartReference (although that would need more investigation). That would allow the IPresentablePart access to the properties before the actual IWorkbenchPart was instantiated. This assumes the properties will be treated like the other properties like title, name, description, icon, tooltip. I think that's a decent model to work from. Another option would be to only allow properties that implement IPersistableElement ... that would force them to support saveState(IMemento) and supply an IElementFactory ID for recreation. I think this design is more restrictive than just using String objects. Later, PW Yea I thought about IPersistableElement too. Just seems to add more complexity/responsibility and in reality it doesn't make that big a difference to have Objects rather than Strings. Another twist, some properties will not be logically persistable. Take my previous example, where the presentation is trying to show an editor as read only because a user doesn't have rights to it. That user could be given read/write rights after that read only property is persisted. So when the workbench is restarted, that value is stale and in fact not correct. But IMO thats ok. Persist the latest value and let the editor re-evaluate it when its actually created. The presentation will just update when the user selects the editor/view. (In reply to comment #8) > [... snip ...] > thats ok. Persist the latest value and let the editor re-evaluate it when its > actually created. The presentation will just update when the user selects the > editor/view. I think that'll work fine. When the part is first instantiated, the reference asks the part for all of its latest values ... and fires a property change event if the part returns new values. PW Paul, is there any movement on this item? Let's see what we can do. PW Created attachment 56661 [details]
part Properties v01
Draft patch of adding arbitrary properties to a part, saving them in the reference between sessions, and showing them through IPresentablePart.
1) there is no default implementation on IWorkbenchPart, this depends on each part implementing IWorkbenchPart3 if they want to store arbitrary properties
There is also the question of what to do with property change events. Right now, there are a few defined constants, but they are integers. That makes it hard for different parts to define arbitrary properties.
One option is to have a mediator that everybody will talk to. Want your property change id, ask the mediator with your key and it'll give you a number. The numbers will be consistent for one session, but change between sessions.
With the mediator it might be able to right a base implementation in WorkbenchPart ... but this might be too much overkill.
More anon.
PW
It seems strange that the id for the property is a string, but the id for the property event is a int. Perhaps we could combine them? Make the property keys be ints? I thought about making the property keys ints ... but that would make it exceptionally easy for property collision. With Strings we can have keys tack their plugin id on the front. I'm gonna stick with Strings for now, but ask some of the other UI committers. It might be that we just live with constants starting at 0x500, and everybody has to deal with the fact that getProperty(0x500) might return the correct property, a useless String, or null. PW Another option that avoids dealing with the String to int mapper is to say that all of these properties are user properties, and we'll fire IWorkbenchPartConstants.PROP_USER Then there's no mapper, but you wouldn't be able to narrow down which user property has changed. Nick, do you have any opinion about firing the property change event? PW Of course we could go half way, and just fire the hasCode() for that string key ... it would be consistent within one run of eclipse (but you wouldn't be able to save that int id), and the odds of you checking the incorrect property are very small (but not 0). PW Using the hashCode feels like a hack. In the original design of the presentations mechanism, we deliberately did not provide an open-ended properties mechanism, since that would encourage tight coupling between the presentation and the view/editor implementations. We felt strongly that parts should be agnostic of which presentation is presenting them. On the other hand, I do feel that it's nice to have an escape so that use cases that we never envisioned are possible to implement. Chris' security scenario is definitely a reasonable one, and I know of several RCP apps that require something similar. Created attachment 56930 [details]
part Properties v02
After talking to Nick, here is a draft patch that avoids the int property IDs all together. Just use JFace PropertyChangeEvents for the arbitrary event set.
This patch still needs some TLC, but it solves the random ID problem and allows me to add a default implementation to WorkbenchPart.
PW
Essentially separates the two properties completely. Sounds good to me. Created attachment 56982 [details]
part Properties v03
Update javadocs, include view saveState/restoreState.
PW
Created attachment 56997 [details]
part Properties v04
Added some extra session tests and improved persistence.
PW
Released API and tests >20070117 PW I20070206-0010 PW |