Community
Participate
Working Groups
The 3.2m6 version of data binding doesn't support CellEditors for TableViewer. Attaching patch...
Created attachment 42873 [details] CellEditor support The changes for TextObservableValue are built upon the patch attached to bug 135446. The only CellEditor supported in this patch is for TextCellEditor. When I get time I'll provide more implementations of IObservable for other editors. The patch is against the "version3_2_lockdown" branch. This the data binding changes along with unit tests. An example of how to bind editors can be found in the TableScenarios JUnit.
Created attachment 43411 [details] CellEditor support Cleaned up the patch a bit.
This patch is out of date - I cannot apply it against HEAD. Brad, would you be able to update the patch so that I can review it?
(In reply to comment #3) > This patch is out of date - I cannot apply it against HEAD. Brad, would you be > able to update the patch so that I can review it? To be honest that's probably a good thing. Over time I've become more dissatisfied with the solution. Also it didn't account for editors without controls (CheckboxCellEditor). I'll try to get it cleaned up sometime this week though to at least have somewhere to start.
Created attachment 45272 [details] CellEditor support Rewrote the previous patch. Included in this patch: * support for binding TextCellEditor (editor with control) * support for binding ChekboxCellEditor (editor without control) * convenience mechanism for binding of TableViewers * tests
I just realized attachment 45272 [details] doesn't expose the observables or the ability to retrieve the selection observable. I'll get to that some night this week.
Created attachment 45438 [details] CellEditor support I think the only thing left is figuring out how to allow for TIME_EARLY with a TextCellEditor. Boris, if you have time I'd appreciate it if you could review this one.
Marking as depends upon 149378 as this will be necessary to be able to bind a TextCellEditor early.
Comment on attachment 45438 [details] CellEditor support Because of the API refactoring the patch is no longer valid.
As per the data binding call on Nov. 30th we'd like for this to use 3.3 APIs but also be able to allow this to work with 3.2 viewer APIs.
I spoke with Tom on IRC on Friday a bit getting the new 3.3 viewer editing APIs working with Data Binding 1.0. We're not going to be able to get DB API for the 3.3 viewer API into DB 1.0 but we should still have the conversation of what is needed to setup cell editors to use bindings and also provide a little feedback on the new API. I had previously written cell editor support for tables for the Eclipse 3.2 DB API but our API has changed so much that this is no longer valid. But I took a look today at the 3.3 viewer APIs and am including my initial thoughts on how to bring the 2 together and any difficulties we might have in doing so. DISCLAIMER: I'm not an expert on the new viewer APIs and looked at it for the first time Friday. So the core assumptions of my view could be off. I also haven't written any code validating that my assumptions are all correct and this would work. I'm pretty sure it would but there could be unexpected bumps in the road. In a perfect world here is how I would see this working... class Person { String name; int age; //getters and setters... } class AgeValidator implements IValidator { public IStatus validate(Object value) { Integer i = (Integer) value; if (i.intValue() < 0) { return Status.CANCEL_STATUS; } else { return Status.OK_STATUS; } } } TableViewer viewer = new TableViewer(table); //1 BeanTableViewerColumn column = new BeanTableViewerColumn(viewer, SWT.NONE, "name"); //2 column = BeanTableViewerColumn.asEditableColumn(viewer, SWT.NONE, "age", new AgeValidator()); //create content provider and set input.. 1. Creates a column that knows what property to display in the column. This would look up the converter from DefaultBindSpec or could accept an IConverter. 2. Creates the age column associating the validator to use when validating input. Behind the scenes it would create an instance of a new class BeanEditingSupport that would use the validator and converter for the column to handle validation and conversion of the user entered value. Also by providing this at the column level validation could be invoked from the label provider to determine color, font, etc. of the cell. In BeanEditingSupport behind the scenes when a CellEditor is activated the BeanEditingSupport instance would create a new Binding of the CellEditor and the element and property being edited. It would associate the converter and validator with the Binding that were passed on column construction. Then when the CellEditor is deactivated the BeanEditingSupport would handle validation errors and the applying of the value to the element. The problem with this is that this won't be possible with the EditingSupport API today. EditingSupport doesn't encapsulate the element being edited. It provides API to interact with an element but doesn't know about the editing lifecycle. This knowledge is down in ColumnViewerEditor and is only available to the viewer implementations. So my question to Tom and Boris is is there a possibility of tweaking the EditingSupport API so that instead of being an adapter to an element it could instead be an adapter to the element being edited? I think this would involve moving some of the logic from ColumnViewerEditor into EditingSupport. I know we're in M6 and API is close to being frozen but I'm thinking blue sky here. Let me know what you think.
I think this is difficult to design because there are some conceptual mismatches between DataBinding and the EditingSupport/CellEditor API. I think I can clarify the issues by breaking down the responsibilities of each API role in the two frameworks and contrasting the division of labor. I'll start with DataBinding API, since that's what the people on this bug are more familiar with. IObservableValue 1. Unifies the get/set API for both targets and models via getValue()/setValue(Object) 2. Provides "push" updates for one target or model 3. Unifies the Observer pattern API for both targets and models via IValueChangeListener 4. Unifies the type system API for both targets and models via getValueType() IConverter 1. Implements one-way conversion between target and model or vice-versa IValidator 1. Implements one-way validation between target and model or vice-versa Binding 1. Manages updates in both directions between one target and one model 2. Applies configurable IConverters in both directions 3. Applies configurable IValidators in both directions and at configurable points in the editing life-cycle 4. Provides access to validation status results DefaultBindSpec 1. Implements a registry of automatic/default validators and converters for Binding DataBindingContext 1. Manages creation/disposal of a set of Bindings 2. Bindings are created between individual properties on targets and models 3. Provides centralized access to all validation status results for collected Bindings The DataBinding API allows all participants to be configured independently, and it is almost entirely symmetrical with respect to target and model. IMHO, this is a clean design that separates the concerns well. (Good job, folks!) On the other side: EditingSupport 1. Unifies the get/set API for models only via getValue(Object)/setValue(Object,Object) 2. Provides passive updates for many models with one instance 3. Does NOT implement an Observer pattern 4. Conversion and validation are implicit in get/set API, with no separate type system API 5. Coupled with target API via getCellEditor(Object) CellEditor 1. Unifies the get/set API for targets only via getValue()/setValue(Object) 2. Provides "push" updates for one target 3. Unifies the Observer pattern API via ICellEditorListener and IPropertyChangeListener 4. Conversion and validation are implicit in get/set API, with no separate type system API 5. Validation lifecycle for both directions is coupled with Observer pattern 6. Manages creation/disposal (activation/deactivation) for the target side of a set of bindings 7. Manages target side of bindings for multiple properties in one instance 8. Provides access to validation status results 9. Unifies dirty-state tracking API for targets 10. Unifies clipboard, selection, and undo API for targets 11. Unifies construction and layout API for targets As you can see, the CellEditor API does both more and less than DataBinding, and the various concerns tend to be more highly coupled within fewer objects. EditingSupport represents a central point of control for the whole process, similar to the way that DataBindingContext is the central manager for DataBinding. It should be possible to use it to divvy out portions of the DataBinding API to the appropriate places in the CellEditor API. CellEditor concerns 10-11 will need to be layered on top of the DataBinding API somehow. These are only implemented for Text and CCombo targets in the CellEditor API, so this would not require a major effort to replicate. Adding SWTObservables.observeSelection(Text) would be helpful. I'm formulating a more complete design plan based on these ideas, and I'll post that shortly.
More observations: The DataBinding API is designed to allow targets to exist in an "invalid" state for an arbitrary amount of time. Both DataBindingContext and Binding provide APIs that allow clients to observe or query the validation state and do arbitrary things with that info. CellEditors are designed to track a model for a much shorter period of time, then move on to edit a new model. The validation state and contents of a target are lost as soon as a new element is selected. There is no direct equivalent to a DBC that aggregates validation state for a whole element nor the whole Control. If one existed, it could not rely on the CellEditors to maintain their own state, in the way that DBC relies on Bindings to maintain theirs. Therefore, any DataBinding adapter around the CellEditor API would need to support Bindings with very short lives. The Binding would only exist between CellEditor.activate() and .deactivate(). When a Binding is disposed, the validation state in the DBC would need to act as though that Binding changed to be valid. That way, one DBC will be able to track the overall validation state of the Control while Bindings are being created and disposed dynamically.
(In reply to comment #12) Hi Peter, > I can clarify the issues by breaking down the responsibilities of each API role > ... Awesome! I couldn't have said it any better. If you don't mind, I would like to use this for the EclipseCon long talk. As for cell editing and data binding - let me write down some of my thoughts. After our (IMO failed) attempts at coming up with a bind(...) API for connecting a model and a table viewer, I am now quite happy with our current solution, even if it not complete yet. The data binding framework gives you generic content and label providers that bridge between observables and JFace viewers. No need to go through the data binding context, and no need to use a binding. For cell editing, I am not sure how best to do it. If you don't need conversion or validation, you can probably write a generic EditingSupport subclass that works in terms of the observable map(s) on the model side. Adding conversion is not difficult, but for validation it is not that easy: First of all, you probably want to make validation results available to the data binding context. Then, as you point out, if the user entered an invalid value, you need to revert back to a valid value, or somehow store the invalid value temporarily. Reverting back to a valid value is what you described in your last comment - creating and disposing bindings dynamically. In situations where the table data is fully backed by an observable map per column, you could probably implement a solution where you allow longer-lived bindings. For this, you would keep a map from elements to invalid values (per column). The label provider would have to be aware of this as well, taking values from this map if one exists.
Boris, reuse whatever you like from my comments. I aim to serve. :-) As I see it, the primary motivation for blending the two APIs is to reuse the existing CellEditor implementations for the target side (to handle concerns 9-11 above), but use the more convenient DataBinding APIs for validation and conversion. This would be the motivation for people who like the DataBinding APIs for form-like UIs and want to use a similar approach for inline editing of Tables and Trees. I think this is attainable in the short term. 1. Decorate EditingSupport to behave as an IObservableValue, where the "value" is the active element from the viewer. The change events should be generated when getCellEditor(Object element) is called. getValue() should return the current element, and setValue should be noop. (Implementing proper setter semantics would be major pain for minor gain.) This takes advantage of an AFAIK undocumented fact that getCellEditor(element) is always called before getValue(element) or setValue(element). If necessary, we can also send change events if a new element appears in one of those methods unexpectedly. 2. Use the ObservableEditingSupport above as the master in a call to MasterDetailObservables.detailValue(master, detailFactory, detailType). The EditingSupport getValue/setValue can delegate to the resulting detail observable. This leads to the unintuitive situation that ObservableEditingSupport both is-a master and has-a detail (which it delegates to for standard EditingSupport calls). 3. Decorate CellEditor as an IObservableValue. This is the most complex part. For maximum impact, we should automatically detect the standard TextCellEditor, ComboBoxCellEditor, and ColorDialogCellEditor classes and return an appropriate getValueType(). Change events could either be generated via the ICellEditorListener API (for the general case) or could be special-cased for each editor type by listening directly to the underlying editor.getControl(). All other CellEditor methods would delegate to the decorated CellEditor. 4. Use DataBindingContext.bindValue() as normal, using the ObservableEditingSupport.getDetailObservable() for model and the ObservableCellEditor as target. AFAIK, that's all that would be necessary to present CellEditors as standard players in the DataBinding API. Client code might look like this: // Context -- do once per Table DataBindingContext dbc = <...>; TableViewer viewer = <...>; // Bind -- do once per column // Target TableColumn col = new TableColumn(viewer.getTable(), SWT.LEAD); ObservableCellEditor oce = ViewersObservables.observeCellEditor(new TextCellEditor(viewer.getTable())); ViewerColumn vcol = new ViewerColumn(viewer, column); CellLabelProvider lp = <...>; vcol.setLabelProvider(lp); // Model EditingSupport es = new ObservableEditingSupport(oce, BeanObservables.valueFactory("propertyName")); vcol.setEditingSupport(es); // Binding BindSpec bindSpec = <...>; Binding binding = dbc.bindValue(oce, es.getDetail(), bindSpec); For extra credit, we could also provide some interop with the old CellEditor APIs: 5. Delegate CellEditor.getErrorMessage() to binding.getValidationStatus().getValue().getMessage(). Similarly for CellEditor.isValueValid(). This would create the unintuitive situation of an IObservable knowing about its own Binding. The binding would have to be set back on the ObservableCellEditor explicitly. 6. Emulate ICellValidator via the IValidators set on the Binding, so that CellEditor.getCellValidator() returns a usable object. Set this as the active ICellValidator on the decorated CellEditor, so that the ICellEditorListener methods act as expected. With those tweaks in place, client code relying on standard CellEditor behavior should still function properly. The only exception would be CellEditor.setCellValidator(), which would have to be a noop. This is unlikely to be a problem, since the BindSpec APIs provide a superset of the functionality.
After a bit more thought, I realized that my proposal sets up two redundant data flow paths for changes from target to model. The standard CellEditor API will call EditingSupport.setValue(element, value) during CellEditor.fireApplyEditorValue(). The DataBinding API will call model.setValue(value) for the same change. The "updating" flag might catch this already, but we should verify that we can't trigger an infinite recursion.
(In reply to comment #15) > As I see it, the primary motivation for blending the two APIs is to reuse the > existing CellEditor implementations for the target side (to handle concerns > 9-11 above), but use the more convenient DataBinding APIs for validation and > conversion. Add to that the reuse of the metadata that is provided to reduce the redundency of the setup. This then opens up the ability to default editors. In the previous version of the support I had to hack around the behavior that the viewer get/set the value in the cell editor. My reason for bringing this up again is that I'd like to see this fixed before the viewer API is frozen for 3.3. If the editing lifecycle was moved from the viewer into EditingSupport it would allow the consumer to use whatever binding library they like as the life cycle would be in an implementation that is exposed to the consumer. The only thing that is viewer specific about the lifecycle are the events to activate and deactivate the editor. Also the API isn't going to get implemented in Data Binding before 1.0 as I don't have time. I'm asking that the viewer API be changed so that when we get to it we have the hooks and don't have to hack around this once again.
We are currently in the process of making CellEditor activation more comfortable and we haven't finalized yet. So I could only second Brad if JFace can do things that would make databinding easy we should do this. The first step from my side was that I move great bunches of code from ColumnViewerEditor to EditingSupport for now. If someone's interested in what's going on in JFace attach yourself to the bug #172646.
(In reply to comment #18) > We are currently in the process of making CellEditor activation more > comfortable and we haven't finalized yet. So I could only second Brad if JFace We have still not finalized but I just uploaded a fairly feature complete version so if you have some input give it now before it's too late.
For anyone interested in editing support a snippet[1] is available demonstrating how this is done with JDB and the 3.3 Viewer APIs. This bug will remain open until we incorporate this into the library. [1] http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.jface.examples.databinding/src/org/eclipse/jface/examples/databinding/snippets/Snippet013TableViewerEditing.java?view=markup
Created attachment 82061 [details] ObservableValueEditingSupport implementation WIP...
Created attachment 82072 [details] Column builder prototype (requires attachment 82061 [details] patch) I hacked around a bit to get a few ideas out on how to make building viewers, using Bindings, simpler. The code attached is all prototype, untested, and all around bad but it shows where I was going with this. The API that I'm concerned with is the following (lines 202 - 207)... BeanTableViewerBuilder builder = new BeanTableViewerBuilder( peopleViewer, peopleViewerContentProvider.getKnownElements(), Person.class); builder.column("name").withTextEditor(bindingContext).build(SWT.NONE); builder.column("OS").build(SWT.NONE); Let me know what you think.
Thanks for the invitation to this discussion. I am not to deep into the DB internals. I understand the validation problem. To that point I would really cache the whole table content ? i.e. implement a transparent layer between the viewer and the actual model. I come from the real world api view, as I implement business apps on eclipse. From that point of view I always have a domain model thats not a simple bean but an object graph. One common aspect is that we have for e.g. ValueLists that are input for most of our combos. For now I dont use inline editing as its to complicated and always open a dialog for editing. Thats ugly and I would love to do inline editing. A common scenario is class Person{ String name; Value type; } where Value is a selection from ValueList. More complex cases with deeper graphs are obvious. So I need to have a Combo (or whatever as combo has some disadvantages too, one can enter anything in the textbox) thats bound to the type property and is initialized with a set of values from the corresponding ValueList. I would like to see an simple api that binds my domain model to the tableviewer where I can give as simple as possible instructions on how to edit each column.
Released "ObservableValueEditingSupport implementation" to HEAD.
Hi I had a quick look at this today. I guess I will need the ComboBoxViewerCellEditor too :) But I have I little question. What I did is the following: m_bindingContext = initDataBindings(); countCol.setEditingSupport(new CountColEditingSupport(tableViewer,m_bindingContext)); // add a listener to each binding to mark the editor dirty ValidatingContext.addDirtyListenersToContext(m_bindingContext, (DirtyListener)getEditor()); seems that this doesnt touch the binding of the cell editor. I guess thats even right as the binding should be created on demand. But what would you recommend me to do to get the dirty state of my editor ? Do I have to go down to the model elements and listen on them ? It was so easy with the bindings :) Thomas
(In reply to comment #25) > // add a listener to each binding to mark the editor dirty > ValidatingContext.addDirtyListenersToContext(m_bindingContext, > (DirtyListener)getEditor()); You probably need to track the observable list m_bindingContext.getBindings(), and add/remove dirty listeners to bindings as they are added/removed to/from the list of bindings.
*** Bug 215509 has been marked as a duplicate of this bug. ***
