[1/2] Databinding binds to deprecated classes (Properties Classes) #323

[col-panic]

This PR updates the Properties class references to the new locations. It only implements a partial solution of the deprecated class databinding. For the properties files the replacement is quite simple, as the only change (wrt to windowbuilder) is the package renaming.

• org.eclipse.core.databinding.beans.BeanProperties -> org.eclipse.core.databinding.beans.typed.BeanProperties
• org.eclipse.core.databinding.beans.PojoProperties -> org.eclipse.core.databinding.beans.typed.PojoProperties
• org.eclipse.jface.databinding.swt.WidgetProperties -> org.eclipse.jface.databinding.swt.typed.WidgetProperties
• org.eclipse.jface.databinding.viewers.ViewerProperties -> org.eclipse.jface.databinding.viewers.typed.ViewerProperties

Bound for separate PR as the implementation substantially differs, and a drop-in-replacement is not easily possible:

• org.eclipse.core.databinding.beans.BeansObservables.observe* -> reimplement required functions in org.eclipse.core.databinding.beans.typed.BeanProperties
• org.eclipse.jface.databinding.swt.SWTObservables.observe* -> reimplement required functions in org.eclipse.jface.databinding.swt.typed.WidgetProperties
• org.eclipse.core.databinding.beans.PojoObservables? -> reimplement required functions in org.eclipse.core.databinding.beans.typed.PojoProperties
• org.eclipse.jface.databinding.viewers.ViewersObservables -> reimplement required functions in org.eclipse.jface.databinding.viewers.typed.ViewerProperties

General info
see https://bugs.eclipse.org/bugs/show_bug.cgi?id=546820
see https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=4f25fbf41f9411ded14d3cec0e951e7bcaf51319

[github-actions]

Unit Test Results

    4 files      4 suites   1m 28s ⏱️
  83 tests   83 ✔️ 0 💤 0 ❌
166 runs  166 ✔️ 0 💤 0 ❌

Results for commit fd3d6d3.

[col-panic]

Replacing the Observables seems to be much more complicated. Take the following example

IObservableValue passObserveWidget = SWTObservables.observeText(passText, SWT.Modify);
IObservableValue passObserveValue = BeansObservables.observeValue(mySampleBean, "pass");

should become

ISWTObservableValue<String> passObserveWidget = WidgetProperties.text().observe(passText);
IObservableValue<Object> passObserveValue = BeanProperties.value("pass").observe(mySampleBean);

the problem is the split into required submethods. This does not seem to easily match the existing structure (as far as I understand it up to now). A rather easy approach would be to offer methods like WidgetProperties.observeText(...) such that it becomes a drop-in replacement.

[wimjongman]

so you say that Eclipse dropped data binding, and there is no drop-in replacement.

[col-panic]

so you say that Eclipse dropped data binding, and there is no drop-in replacement.

uhm, don't know?! I just see, that updating WindowBuilder gets much more complicated from this point on.

[wimjongman]

A rather easy approach would be to offer methods like WidgetProperties.observeText(...) such that it becomes a drop-in replacement.

Thanks for the analysis. This has to be changed in Eclipse data binding correct?

[merks]

Couldn't one write one's own utility class for this purpose?

[wimjongman]

Couldn't one write one's own utility class for this purpose?

I don't see how. All WindowBuilder generated classes are SWT/JFace pojos.

[merks]

I see, so there are no helper libraries involved at all just purely the generated classes and the libraries of the targets. Sorry for the noise.

[wimjongman]

I see, so there are no helper libraries involved at all just purely the generated classes and the libraries of the targets. Sorry for the noise.

No noise, we can use all the ideas we can get.

I filed issue: eclipse-platform/eclipse.platform.ui#426

[col-panic]

As commented in eclipse-platform/eclipse.platform.ui#426 - if there are going to be new Classes/Functions to be added in the next release, it might already might sense to do a partial commit. The existing code is wrong anyway, so at least some parts could be fixed.

[vogella]

@col-panic can you finish the commit with the existing drop-in replacements so that we can already support part of the scenarios?

[col-panic]

@vogella I think so. will do some more checks and open a new issue for the missing parts then.

[col-panic]

@vogella @wimjongman please have a review on the state of this PR. I want to separate works on the missing part, as this is already confusing enough to me. Besides, the parts fixed in here are already a partial solution - for what my manual tests were worth.

[wimjongman]

Hey @col-panic. These changes look good to me. Thank you. Did you test if the new bindings can also be parsed back again?

[col-panic]

yes, thats the state so far

not sure about adding type safety right now.

[wimjongman]

Okay great. Can I squash this, or is that inconvenient for you?

[col-panic]

sure, no problem!Von meinem iPhone gesendetAm 05.11.2022 um 09:40 schrieb Wim Jongman ***@***.***>: Okay great. Can I squash this, or is that inconvenient for you? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[col-panic]

For advanced databindings, there are even methods missing, that got removed from the resp. type counterparts. One example is https://github.com/eclipse-platform/eclipse.platform.ui/blob/eb5f64917709bd7c903f72569d1344fbeda305bc/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/databinding/swt/WidgetProperties.java#L238 which is used in radio button databinding

[wimjongman]

Oops. I think that information should go to platform.

[col-panic]

I'm not sure, this all seems to go a lot deeper, and seems quite overwhelming. There is a lot of code, due to the following setting

vs

(second is the default)

which allows to configure code generation between the observable and the properties style.

As far as I understand this now, the Observable way is generally not taken anymore (and in the future not available anymore) - so there seems to be a lot of stuff that makes sense to be cleaned up. And some code I tested still wants to simply generate the bindings old-school as it was never updated.

So it might sense to first remove this entire option, and migrate everything to new style. There are also so many scenarios where I don't know how to test extensively. This all seems to be a really big pile of work, if done right.

@vogella do you probably have an "uber-databinding" sample that includes all possible widgets and a fitting model which could then be used for testing the databinding generation? It was me simply trying to databind checkButtons where I found out about the WidgetProperties#selection not being available anymore ...