Bugzilla – Full Text Bug Listing
| Summary: | [DataBinding] The generics type parameters need to be specified in data binding classes | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Simon Clarkson <simone.clarkson> |
| Component: | UI | Assignee: | Stefan Xenos <sxenos> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | b.michael, bokowski, chrislake, chriss.dev, daniel_megert, eostroukhov, holger.seith, jawr, jens, jpp-mac, Lars.Vogel, mallo.ovidio, mike, Mike_Wilson, nigelipse, prakash, pwebster, skrap, sxenos, tom.schindl, zeb.ford-reitz |
| Version: | 4.1 | ||
| Target Milestone: | 4.6 M4 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| See Also: |
https://git.eclipse.org/r/50300 https://git.eclipse.org/r/50299 https://git.eclipse.org/r/51944 |
||
| Whiteboard: | |||
| Bug Depends on: | 472672, 472673 | ||
| Bug Blocks: | 470551, 472674, 478524, 531748, 546978 | ||
| Attachments: | |||
|
Description
Simon Clarkson
Generics can be added without loosing pre 1.5 support because from a bytecode level this is no problem. Equinox is doing this in 3.7. The problem was/has been until now that nobody had the time/resource make this big investment. I often tried to raise funds for work on JFace, ... but because nobody ever did I left 3.x. If you'd ask me and you want Eclipse Databinding to be Generics aware I'd say you should do this at Eclipse. Tom, how do you think that it would be possible to introduce generics to the databinding framework? Even if the bytecode is backward compatible, would it be possible (from Eclipse's API point-of-view) to introduce generics on the existing plugins or would we have to fork them and maintain two versions of the plugins? Of course, it would be great to have generics for databinding (and Eclipse in general...). (In reply to comment #2) > Tom, how do you think that it would be possible to introduce generics to the > databinding framework? Even if the bytecode is backward compatible, would it be > possible (from Eclipse's API point-of-view) to introduce generics on the Yes because it is not breaking any backwards compability. Just like your Java 1.4 code magically still worked even that the JDK introduced generics e.g. in List, ... . Naturally we'll have to do it correct. As said Equinox did that in 3.7. > existing plugins or would we have to fork them and maintain two versions of the > plugins? Of course, it would be great to have generics for databinding (and > Eclipse in general...). No need for a fork. Java Generics are defined in a way that you are not break source or binary compability. (In reply to comment #2) > Tom, how do you think that it would be possible to introduce generics to the > databinding framework? I'll try to come up with a starting example but as outlined in my first reply to Simon I'm not able to be the primary driver for this because I'm not having enough unpayed spare time to work on it beside that it is too late for 3.7 because M6 is in about 6 weeks. My understanding is that code using generics can be built byte-code compatible with 1.4 but not 1.3, so if the project lead is willing to drop Java 1.3 support in data binding for Eclipse 3.8 then there is no reason to fork.
I can wait for Tom's starting example or if he does not have time then I would be very pleased to put together a patch to start this process for Eclipse 3.8. The most useful areas for me would be:
1. WriteableValue, WritableList, WritableMap, WritableSet
2. ComputedValue, ComputedList, ComputedSet
However I suspect, from my experience adding generics to other projects, that it will really end up being an all or nothing change. Once one starts adding generics it is hard to stop.
One area that is not clear to me is when dealing with observables provided by BeansObservables and PojoObservables. In order to do this in a type-safe manner the calls will require the class to be passed. BeansObservables.observeValue, for example, will need an extra parameter being the class. That will work if the bean property is of a type that is not parameterized. If the bean property is parameterized then we will not be able to use reflection but must instead provide method implementations to get and set. For example, consider the bean:
class Bean {
List<String> getFoo();
void setFoo(List<String>);
}
Something like:
BeansObservables.observeList(myBean, "foo", new ListProperty<Bean, String>() {
@Override
getList(Bean bean) {
return bean.getFoo();
}
@Override
void setList(Bean bean, List<String> value) {
bean.setFoo(value);
}
}
I haven't looked at the code in a while so I don't know exactly how this fits in but the gist is that property accessors would need to be written for each property instead of using reflection. I think this can be added to the IValueProperty, IListProperty etc implementations.
It would also be useful if problem areas can be identified.
(In reply to comment #5) > My understanding is that code using generics can be built byte-code compatible > with 1.4 but not 1.3, so if the project lead is willing to drop Java 1.3 > support in data binding for Eclipse 3.8 then there is no reason to fork. > Correct. > I can wait for Tom's starting example or if he does not have time then I would > be very pleased to put together a patch to start this process for Eclipse 3.8. Don't wait for me to act - I've plenty of other things I'm currently heavily invested! > The most useful areas for me would be: > > 1. WriteableValue, WritableList, WritableMap, WritableSet > 2. ComputedValue, ComputedList, ComputedSet > > However I suspect, from my experience adding generics to other projects, that > it will really end up being an all or nothing change. Once one starts adding > generics it is hard to stop. I think we need to have an all or nothing! > > One area that is not clear to me is when dealing with observables provided by > BeansObservables and PojoObservables. In order to do this in a type-safe > manner the calls will require the class to be passed. BeanObservable and PojoObservable is deprecated - we should concentrate on the Properties-API. Created attachment 189176 [details] patch to add generics to o.e.core.databinding.observables Well, here is my initial attempt. I started with o.e.core.databinding.observable. Follows the usual pattern, use types only when needed but allow Objects when type restrictions are not strictly necessary. For type names I have followed the pattern in the Java collections. I have also used <M> for the type of the master record in master-detail classes and <I> for the type of the intermediate keys/values in CompositeMap. This work did show one bug which I have reported as bug 337378. The static helper methods to create empty observable lists and sets cannot return a typed collection. They are parameterized with <Object>. This ensures backward compatability and users can call the constructor directly if they need it to be parameterized. BidiObservableMap. This contained a map that mapped either value (type V) to a key (type K) or to a set of keys (type Set<K>). In order to keep the typing clean I created two maps, one contains the value if a single key and the other map contains the value if a set of keys. The alternative would have been to always create a set of keys even when there is only one key. I decided not to do that because that would be less efficient in the majority of cases where there is only one key, and I assume it was for efficiency that a set of keys was not created everytime when this code was originally written. The other alternative would be a map from V to a class that contains two fields, a K and a Set<K>. That would avoid two map lookups. However on the assumption that most values will have only a single key there are two lookups only in a minority of cases (if an entry is found in the map to keys then we don't bother to look in the map to sets of keys). addMapping seems to accept a set as a key. Not sure why, and I think the code was broken if the keys happened to be a sets themselves (not likely, I know). This is a private method and sets of keys are never passed so I just removed that code. UnionSet used an array for the child sets which I changed to a HashSet. This was done because arraYS of parameterized types are not allowed. toArray in IdentitySet, IdentityMap etc were changed a bit. These are not type safe and I don't think it is possible to implement these methods in a type-safe manner which is a shame. I am concerned at how this is going to be committed because 1. It is a lot of code for a committer to review and could take longer to review that it took to make the changes. So a committer is really just going to have to trust that there have not been code changes except in the areas identified by me above. I would also be concerned if this did not get committed reasonably quickly because there could otherwise be a lot of merging to do. Another issue is that I believe we are too late for 3.7 which means this needs to go into 3.8 code. I don't know if 3.8 has been forked yet from the 3.7 builds. These changes are based on the copy in git. The listeners and diffs have not been done. That would be the next step and most of the warnings will go away when that is completed. There are now around 300 warnings in the observables bundle (down from a peak of almost 1000). I would be very happy to complete the work but I would like to see the work already done be committed before going any further, or at least to know what the plan for committing is. I don't think a CQ will needed because, although a lot of lines have been changed, it is a routine upgrade with nothing original. If you disagree though I would be happy to submit a CQ. Matthew, Boris, what do you think would be the best approach for this? Even if we're not at M6 yet, I would say that it is too late to get this done for 3.7. If we do this for 3.8 and want to start with this now, we would probably have to make a BRANCH of the databinding plug-ins but this would of course imply a certain overhead for any change to the databinding code until the end of the 3.7 release cycle. I'm not even sure whether it's common practice to make such a BRANCH during the API freeze period in order to still keep things running? Wow Nigel! Very thorough.
Here are my comments as I go through the patch:
* Observables.emptyObservableList could be typed as "<T> IObservableList<T> emptyObservableList()", similar to java.util.Collections.emptyList() returns a list of whatever type is inferred on the left hand side. Ditto for Observables.emptyObservableSet. I think these just got overlooked.
* Looks like we still need to generify the listener interfaces and diff classes. Just putting it here so we don't forget.
* Nice catch on generifying MasterDetailObservables and IObservableFactory to work together. However you could loosen the type requirements a little, e.g.:
public static <M, E> IObservableSet<E> detailSet(
IObservableValue<M> master,
IObservableFactory<? super M, IObservableSet<E>>) // note wildcard
This way if you have an IObservableFactory<Object, IObservableSet<Foo>> you
could still observe the detail of an IObservableValue<Bar>.
* Java 5 has built in IdentitySet<E> and IdentityMap<K,V> classes. If we're still targeting Java 1.4 then we have to keep these in. If we're targeting Java 5 then we can just extend these off the ones in the standard collections library and be done with it. So this decision needs to be discussed further.
I think moving up to 1.5 would make sense, unless we know specifically that there are consumers who are still on 1.4 (I am not aware of any). Created attachment 192745 [details]
patch to add generics to o.e.core.databinding.observables
Above is a patch with my second batch of changes for o.e.core.databinding.observables. This completes the work for this bundle in terms of parameterizing what should be parameterized though there is still quite a bit of tidy up and testing to do. This patch does not include the loosening of detailSet method or the use of IdentityMap and IdentitySet from the Java libraries. I have one question about the toArray methods. As you will note these are the cause of most of the warnings. In my view I don't think there is any need to provide implementations of these methods at all because the methods from the super classes will work just as well. I now need to run this through the tests and also to use in our production code so I can identify potential problems. Created attachment 208901 [details]
fix tests and rebase to current head
I have rebased to the latest head. This includes the new master-detail collections added by Mallo back in February and to which generics have now been added.
The tests all pass. There was one test I needed to change. The test for the immutable list passed "new Object()" and expected UnsupportedOperationException. That actually now gives a cast class exception. Passing a Map.Entry object will give the desired result.
I updated the version from 1.4 to 1.5 because if users add generics then they can't go back.
This works in our production code and I believe this is in a state that it can be committed. If this gets committed then I might feel motivated to look at the other plug-ins, which should be much easier, and to sort out the remaining 23 warnings.
Matt, could you look to see if this could go into M5? PW Just let me know what I can do to help get this into 4.2 M6. Matt did you ever get a chance to look at this again? The data binding project is dead. There has not been a commit to the project for well over a year, although many bugs in it, features, patches. I don't think any of the committers are involved with the project any more. I don't know what the dead project procedures are at Eclipse but in the short term we need to fork so we can continue to get bug fixes, generics and all the rest. Please put the generics version on GitHub or somewhere and build the jars so I and others can use them. It's nominally under Platform UI, but you are correct no one is actively looking at these plugins. If you (or someone) is interested in becoming a databinding committer (providing code fixes and accepting community contributions) you can post an email to platform-ui-dev@eclipse.org with a little about yourself and how you would like to see databinding go forward. For the technical side of getting involved, see http://wiki.eclipse.org/Platform_UI/How_to_Contribute The repo that contains databinding is already on github, under http://github.com/eclipse PW I empathize with your position, Simon. Let me offer an alternative viewpoint, and a proposal. 30 months ago I switched jobs, and data binding in SWT+JFace was no longer part of my everyday work. For a little while I continued to support the project and fix small bugs in my spare time. However my motivation to participate has waned. There are a couple good reasons for that. 1. It's no longer part of my job description. I am authorized by my employer to work on the project, however I cannot use work time to do so except in rare situations when I have no tasks. 2. The API is painted into a corner. For several years we have doggedly maintained backward compatibility, and continued to support some old, poorly thought-out APIs that have turned into a "spec once, implement everywhere." Realms are a perfect example of this. 3. The Eclipse Platform development schedule and policies (e.g. code freezes) are, in my opinion, not conducive to part-time or amateur contributors. Maybe it's just Murphy's Law working against me, but it seems every time I get really motivated to work on things and fix bugs, it's a damn milestone week again. So even if I code up something, I can't push it back to the repository. 3a. We may have adopted Git with all its great branching and merging abilities. However we are still using the same old CVS branching model that requires everybody to stop checking in code whenever it's time to cut a release. With tools like Git, this should not be necessary [1]. I would very much like to move this project forward. However within the confines of the Eclipse process, I do not have sufficient autonomy to accomplish anything meaningful in my limited spare time. Eclipse Data Binding needs to be gutted and rebuilt. I propose we fork Eclipse Data Binding at Github. Having built a few new different data binding libraries in the past few years, I think I have a pretty good vision of what API we should shoot for [2]. The long-term goal is still to have the project live at Eclipse. But for the sake of rapid iteration, I think we should leave the reservation for a while. We can continue to enforce the same IP constraints (e.g. obtain positive declaration of ownership and licensing when contributions exceed 250 LOC). This should help ensure that the project will be IP clean to contribute back when we are ready. [1] http://nvie.com/posts/a-successful-git-branching-model/ [2] http://code.google.com/p/bindage-tools/ (My own project: flex data binding for AS3) (In reply to comment #19) > > 3. The Eclipse Platform development schedule and policies (e.g. code freezes) > are, in my opinion, not conducive to part-time or amateur contributors. Maybe > it's just Murphy's Law working against me, but it seems every time I get really > motivated to work on things and fix bugs, it's a damn milestone week again. So > even if I code up something, I can't push it back to the repository. we always allow pushes to topic branches, even during milestone week. Same applies to 3a, work can continue even during a freeze, just not on the main line. > Eclipse Data Binding needs to be gutted and rebuilt. > > I propose we fork Eclipse Data Binding at Github. Having built a few new > different data binding libraries in the past few years, I think I have a pretty > good vision of what API we should shoot for [2]. While I'm not saying this is a good idea or bad idea, if you want to fork to evolve the API on a "lighter" timeline you should consider the Platform E4 incubator project (some extensive core.resources experimental features were done in E4 with forks before being consumed back into the main repo). Some of the advantages of using E4 is 1) still built and signed at the foundation, 2) it's easy to become a committer on E4, and 3) contributions and committers have already signed up for the eclipse IP guidelines making it easy to roll work there back into the fold, so to speak. PW (In reply to comment #20) > Some of the advantages of using E4 is 1) still built and signed at the > foundation, 2) it's easy to become a committer on E4, and 3) contributions and > committers have already signed up for the eclipse IP guidelines making it easy > to roll work there back into the fold, so to speak. > +1. It would be easy for me to put my changes into a Github fork but it seems like the consensus is that data binding is moved to e4. So assuming the extra bureaucracy can be completed, I'll plan on checking the changes in there. Matthew, you say that data binding needs to be gutted and rebuilt. Can you document the details? How would the ideal APIs look if you did not need to worry about backwards-compatibility? (In reply to comment #22) > It would be easy for me to put my changes into a Github fork but it seems like > the consensus is that data binding is moved to e4. Right now it's an offer for Matthew to consider. Just to give you an idea of the bureaucracy if e4 is a good place to re-work databinding: 1) setting up the e4 fork for databinding to work on should take very little time 2) becoming an e4 committer involves posting you would want to work on the databinding project. Then there are committer elections, to get approval (easy, just takes 7 days). Then you have to fill in the committer paperwork (your current employer may or may not make it easy for you), and get an ID. Once the fork is in e4 the e4 version is also on github, and committers can accept contributions from other github forks (bugzilla can be used for a pull request). That's more or less where the process is now. PW Some time ago I wrote up a google doc with the details: https://docs.google.com/document/d/1QmH4ZPTqXd8k6ammw9nJBVqF-oVELqisHySPpf6bFi8/edit (In reply to comment #22) > Matthew, you say that data binding needs to be gutted and rebuilt. Can you > document the details? How would the ideal APIs look if you did not need to > worry about backwards-compatibility? Sorry I failed to provide context with my previous comment. The Google Doc link was in answer to the above question. Thanks Matthew for the link to the documents with all your ideas. I added a couple of comments but then ended up just copying the document and making a few changes. My copy is at https://docs.google.com/document/d/118alIDqhYplbsfddnY6B5YjPO_DiiGuxY-0xObShbs8/edit. It seems the best way forward with the Generics is if someone puts data binding into e4. I'll then check-in the Generics into a fork. Others can build from that and it will be easy to pull into e4. I can also use the fork to play around with the ideas in Matthew's document and see how well they work in a real project. Nigel, I've added your Bugzilla email address as an editor on the document. Could you please highlight your additions in a separate color so I can see which parts are new to the discussion? I am now back involved in a project that could really benefit from the discussed work and I also have time that will allow be to make considerable progress on this. It does not appear that Databinding has been moved into the e4 repository. Paul Webster said that was easy to do. If there is anything I can do to help that happen please let me know. Once that is done I will can start work. Paul also mentioned that I should get IP approved so I can commit to a fork from which you can pull without IP issues There won't be issues from my employer, I've had committer status before on another project. I tried editing Matthew's document in googledocs. Despite creating an account with the same e-mail as I use in Bugzilla, I did not appear to have edit rights. However the changes essentially boil down to: 1. It should not be necessary to have to provide both a validator and a converter if the validator is only checking that the convertor can proceed. The converter itself should raise the errors directly. This makes it simpler for the user. It also makes more sense from an implementation point-of-view because to validate one often has to do the conversion and check for errors, then drop the converted value, then do the conversion again in the converter. 2. I don't think it is necessary to use the twoWay method to do two-way bindings. The same form used for one-way binding could in fact be used for two-way binding. This is also safer and simpler for the user because they only specify one convertor which can handle conversions in both directions. 3. You are correct to separate out the 'binding' from the 'observables', so, for example, delayed binding is considered a different form of binding, not implemented as an observable. The two are interchangable but by isolating observables to just the actual data we get more flexibility. The 'stale' issue can then be cleaned up. It is also easier to implement other binding forms. An example of another form of binding is the 'default value' binding, which binds together three observables, model, target, and default value. The default value is usually a computed value and is input only to the binding. Some of the changes to the API were to make generics work better. The idea of Functors in particular makes type checking harder. Also, is there a dev mailing list for databinding? It seems platform-ui-dev is the closest that covers databinding. Nigel (In reply to comment #28) > I am now back involved in a project that could really benefit from the > discussed work and I also have time that will allow be to make considerable > progress on this. > > It does not appear that Databinding has been moved into the e4 repository. > Paul Webster said that was easy to do. If there is anything I can do to > help that happen please let me know. Once that is done I will can start > work. Paul also mentioned that I should get IP approved so I can commit to > a fork from which you can pull without IP issues There won't be issues from > my employer, I've had committer status before on another project. If this works for you and Matthew we can start that right now: 1) make sure you and Matthew are committers on the e4 project or start elections for it 2) once that's moving ahead, create an e4 repo to build the databinding bundles you specify. Matthew, what do you think? PW Created attachment 225758 [details]
my changes to add generics to org.eclipse.core.databinding.observable 1.4.0
Created attachment 225759 [details]
my changes to add generics to org.eclipse.core.databinding.property 1.4.0
As requested by Nigel, I’ve just added two patches with the changes I made to add generics to both org.eclipse.core.databinding.observable and org.eclipse.core.databinding.property as published in my github repo https://github.com/jppellet/ . Feel free to use in any way! Thank you Jean-Philippe. Attaching that allows us to use the work you have done and that is especially helpful as o.e.c.d.property does really need generics too. Hopefully we will sort out some committers soon so all this work everyone has done on generics can finally get checked in. In the latest E4 build, we had pom version failures (then version in the pom file needs to be updated when the MANIFEST.MF is). Also, when you start using Generics you need to bump up the Bundle-RequiredExecutionEnvironment to J2SE-1.5 I've released http://git.eclipse.org/c/e4/org.eclipse.e4.databinding.git/commit/?id=3108218db7fadd9bd650f942ccc84b5b3f938da3 to fix that and restart the build PW Ooops, had to update the test plugin as well. [ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:0.17.0-SNAPSHOT:compile (default-compile) on project org.eclipse.jface.tests.databinding: Compilation failure: Compilation failure: [ERROR] /opt/public/eclipse/e4/build/e4/downloads/drops/4.0.0/I20130312-1315/org.eclipse.e4.databinding/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/internal/databinding/observable/masterdetail/SetDetailValueObservableMapTest.java:[143,0] [ERROR] sdom.put(person, "name2"); [ERROR] ^^^ [ERROR] The method put(Object, Object) is ambiguous for the type SetDetailValueObservableMap [ERROR] /opt/public/eclipse/e4/build/e4/downloads/drops/4.0.0/I20130312-1315/org.eclipse.e4.databinding/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/internal/databinding/observable/masterdetail/MapDetailValueObservableMapTest.java:[146,0] [ERROR] mdom.put(person, "name2"); [ERROR] ^^^ [ERROR] The method put(Object, Object) is ambiguous for the type MapDetailValueObservableMap [ERROR] /opt/public/eclipse/e4/build/e4/downloads/drops/4.0.0/I20130312-1315/org.eclipse.e4.databinding/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/observable/list/AbstractObservableListTest.java:[87,0] [ERROR] list.add(element); [ERROR] ^^^ [ERROR] The method add(Object) is ambiguous for the type AbstractObservableListTest.AbstractObservableListStub The errors show up if you have multiple JREs configured in Preferences>Java>Installed JREs PW *** Bug 389515 has been marked as a duplicate of this bug. *** I am very interested in generics for Databindings and JFace, because it would simplify a lot of things. The current API feels very unattractiv without it. Is there any progress on this? Yes, this work is completed. I hadn't resolved this bug because it is still in the e4 project (not yet pulled into Platform), and there is always a little more work to do. However it is currently being used in production environments. To get the changes from source: http://git.eclipse.org/c/e4/org.eclipse.e4.databinding.git/ You can find the builds at: http://download.eclipse.org/e4/downloads/ At the time of writing the latest release build is 0.14, with the p2 repository at: http://download.eclipse.org/e4/downloads/drops/R-0.14-201306242200/repository In this repository you will find all the binary and source bundles together with the databinding feature which is: org.eclipse.e4.source.databinding.feature All feedback will be greatly appreciated! resolving this as it is now available in the e4 project. That's great to hear and I'm looking forward to using them! Nigel, out of curiosity, did you use any of the stuff I attached? Cheers, —J.-P. (In reply to J.-P. Pellet from comment #40) > That's great to hear and I'm looking forward to using them! Nigel, out of > curiosity, did you use any of the stuff I attached? Cheers, —J.-P. Yes, you attached, I think it was, the Observable bundle and the Properties bundle with generics. The Observable bundle had been mostly done by then, but your work on the Properties bundle was of great help. You had done the entire bundle which saved a lot of time, and some of your work on the Observable bundle was incorporated too. Thank you so much for making those attachments. >At the time of writing the latest release build is 0.14, with the p2 repository at: > >http://download.eclipse.org/e4/downloads/drops/R-0.14-201306242200/repository > >In this repository you will find all the binary and source bundles together with the databinding feature which is: > >org.eclipse.e4.source.databinding.feature The Feature "org.eclipse.e4.source.databinding.feature" is not included in the repository. Only the features: - E4 Databinding (Incubation) 0.13.0.v20130503-1433 - E4 Databinding Test/Examples 0.13.0.v20130503-1433 It seems that only "E4 Search" includes Source Features but no other Category. I think every feature should contain an corresponding source feature. Nigel, thanks. My pleasure to have been able to contribute something. Any other such plugin which would badly need generics? >Any other such plugin which would badly need generics? Yes JFace! There was some work done, but i don't know the actual state, see: https://wiki.eclipse.org/Eclipse_Platform:_Implementing_generic_in_JFace_viewers I contacted J.-P. Pellet already via email, for JFace Generics help. I want give some feedback to the generified methods of the class Observables:
public static IObservableXXX<Object> emptyObservableXXX()
it should be -> public static <T> IObservableXXX<T> emptyObservableXXX()
other wise one get an compiler error: cannot convert from IObservableSet<Object> to IObservableSet<ExpectedType>
public static IObservableXXX<Object> emptyObservableXXX(Object elementType)
should be deprecated, and this should be added as alternative:
public static <T> IObservableXXX<T> emptyObservableXXX(Class<T> elementType)
^^ the same should be applied to emptyObservableXXX(Realm realm,Object elementType)
(In reply to Christian Schwarz from comment #46) > I want give some feedback to the generified methods of the class Observables: [...] I agree. Nigel, that's how I've done it in my patch for the observables bundle, feel free to copy from it. Some more Feedback... DataBindingContext.bindValue(..) should return Binding<M, T> not Binding<?, ?> to avoid a unnecessary casting. Last night's build failed:
[ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:0.18.0:compile (default-compile) on project org.eclipse.core.databinding.observable: Compilation failure: Compilation failure:
[ERROR] /opt/public/eclipse/e4/build/e4/downloads/drops/4.0.0/I20131023-2200/org.eclipse.e4.databinding/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/Observables.java:[577,0]
[ERROR] public synchronized void addSetChangeListener(
[ERROR] ISetChangeListener<E> listener) {
[ERROR] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[ERROR] Name clash: The method addSetChangeListener(ISetChangeListener<E>) of type new ObservableSet<E>(){} has the same erasure as addSetChangeListener(ISetChangeListener<? super E>) of type ObservableSet<E> but does not override it
[ERROR] /opt/public/eclipse/e4/build/e4/downloads/drops/4.0.0/I20131023-2200/org.eclipse.e4.databinding/bundles/org.eclipse.core.databinding.observable/src/org/eclipse/core/databinding/observable/Observables.java:[754,0]
[ERROR] public synchronized void addListChangeListener(
[ERROR] IListChangeListener<E> listener) {
[ERROR] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[ERROR] Name clash: The method addListChangeListener(IListChangeListener<E>) of type new ObservableList<E>(){} has the same erasure as addListChangeListener(IListChangeListener<? super E>) of type ObservableList<E> but does not override it
Probably related to this commit: https://git.eclipse.org/c/e4/org.eclipse.e4.databinding.git/commit/?id=025d67bbdd363568a336ab33092801b1b8a5afa5
PW
*** Bug 470217 has been marked as a duplicate of this bug. *** Reassigning to myself -- I'll handle merging the changes into the main repo. Note that the patches no longer apply cleanly, but I spent yesterday merging them and they seem to be good now. I've removed some of the changes - This patch also contained some refactoring to split apart the PrivateInterface private classes into separate classes. This would defeat the purpose of the PrivateInterface pattern which, AFAIK, is there to minimize anonymous classes and speed up classloading. - ListDiff contained a commented-out, faster, version of one of the methods which didn't compile - I fixed and used the faster version. - ListDiff contained a new getDifferencesAsList() method that required all subclasses to implement the getter for the set of differences twice. The idea behind it is good, but we should probably deprecate or remove the old method if we're going to replace it. Also, that cleanup isn't required as part of adding generics and we should do it carefully as its own change. New Gerrit change created: https://git.eclipse.org/r/50300 New Gerrit change created: https://git.eclipse.org/r/50299 WARNING: this patchset contains 4429 new lines of code and may require a Contribution Questionnaire (CQ) if the author is not a committer on the project. Please see:https://wiki.eclipse.org/Project_Management_Infrastructure/Creating_A_Contribution_Questionnaire Not sure if you're talking about Nigel's changes or my changes, but if it helps, I'm of course willing to fill out the questionnaire. (In reply to J.-P. Pellet from comment #54) > Not sure if you're talking about Nigel's changes or my changes, but if it > helps, I'm of course willing to fill out the questionnaire. I think we are fine here, as this code is already in the e4 project. But thanks for the reaction. Maybe you can also have a look at the Gerrit reviews from Stefan and see if you can spot any issues with them? (In reply to Lars Vogel from comment #55) > I think we are fine here, as this code is already in the e4 project. But > thanks for the reaction. Maybe you can also have a look at the Gerrit > reviews from Stefan and see if you can spot any issues with them? I'm afraid that's currently out of the scope of things I can realistically do these months. Apologies! Should be better around October if help is still needed. After going over several iterations of reviews with Sergey, I think I've rewritten every single line of the original patch (and the first patch is almost 10,000 lines long now). New Gerrit change created: https://git.eclipse.org/r/51944 Since this change is growing so large, I've broken it into a number of smaller bugs -- and I've broken up the first patch into two smaller patches. bug 472672 covers the changes to org.eclipse.core.databinding.observable bug 472673 covers the changes to org.eclipse.core.databinding.property bug 472674 covers the changes to org.eclipse.core.databinding There are more databinding plugins, but these are as far as I can take this since it's turned into way more work than I planned for. Really wonderful to see this work being done! Are there plans to also add generics to the EMF databinding bundle (org.eclipse.emf.databinding)? Are there plans to add generics to the Ecore classes? That is, EClass, EStructuralFeature and related classes?
> Are there plans to also add generics to the EMF databinding bundle
> (org.eclipse.emf.databinding)?
> Are there plans to add generics to the Ecore classes? That is, EClass,
> EStructuralFeature and related classes?
Please open a bug for EMF for this. The platform UI team cannot make any decisions for EMF.
I submitted a prototype for the addition of type parameters to the EMF databinding classes. This is the bug report: https://bugs.eclipse.org/bugs/show_bug.cgi?id=531316 The addition of type parameters to the databinding classes is part of a proposed larger change to add type parameters to the main Ecore classes, such as EClass and EStructuralFeature. Hiya, Just trying to understand the progress on this ticket. It seems all the work is being done in https://git.eclipse.org/c/e4/org.eclipse.e4.databinding.git, but Eclipse builds appear to take it's source from http://git.eclipse.org/c/gerrit/platform/eclipse.platform.ui.git. Am I understanding that correctly? Do we know when the code will move to the gerrit repo (if it has to)? (In reply to Chris Lake from comment #63) > Hiya, > > Just trying to understand the progress on this ticket. It seems all the work > is being done in > https://git.eclipse.org/c/e4/org.eclipse.e4.databinding.git, but Eclipse > builds appear to take it's source from > http://git.eclipse.org/c/gerrit/platform/eclipse.platform.ui.git. > > Am I understanding that correctly? > > Do we know when the code will move to the gerrit repo (if it has to)? https://git.eclipse.org/c/e4/org.eclipse.e4.databinding.git was an experiment which is currently not further development. The API was generified by Stefan around the 4.6M4 and this bug has no more pending patches to it. I mark it is fixed in 4.6M4 by Stefan and future work will happen in the bugs marked as blocked by this bug. |