Provide missing Databinding methods for the generified variants of the now deleted non-generic variants

[wimjongman]

We are in trouble over at the WindowBuilder project when the Databinding api deletion #323 PR was committed.

WindowBuilder is one of the most popular Eclipse plugins (consistently in the MarketPlace Top with close to 20.000 monthly downloads): Link is here. People use WB to overcome the complexity of GUI design, especially regarding DataBinding.

The last time our project got in trouble with API deletion, it led to a revision of the deletion policy to delete API early (in M1), so we had time to catch and fix it. The catching part worked, but this deletion has no simple fix.

At this point, there is no drop-in replacement for the old API, which makes it practically impossible for us to fix this.

Take the following example

Copied from here

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.

At this point, there are three options: Revert this change, create a drop-in replacement for the old classes, or drop data-binding support from WB.

To keep WB alive, I request to revert the change.

[laeubi]

@wimjongman I don't fully understand the issue. Is it that WB do not support "chained methods"?

[col-panic]

@wimjongman I don't fully understand the issue. Is it that WB do not support "chained methods"?

yes, I'm trying to make a PR to provide updated databinding capability, but to be honest, the windowbuilder code is not that easy. It's quite obvious on how to replace the existing classes, but It seems much more complicated to change the way the methods are built - which makes everything except a drop-in replacement a big effort.

[laeubi]

Not sure if it possible but can't WB generate a Helper method so

IObservableValue<Object> passObserveValue = BeanProperties.value("pass").observe(mySampleBean);

becomes

IObservableValue<Object> passObserveValue = observerValue("pass, mySampleBean);

private static IObservableValue<Object> observerValue(..) {
  return BeanProperties.value("pass").observe(mySampleBean);
}

dont know if thats even less problematic to solve ...

[col-panic]

Not sure if it possible but can't WB generate a Helper method so

IObservableValue<Object> passObserveValue = BeanProperties.value("pass").observe(mySampleBean);

becomes

IObservableValue<Object> passObserveValue = observerValue("pass, mySampleBean);

private static IObservableValue<Object> observerValue(..) {
  return BeanProperties.value("pass").observe(mySampleBean);
}

dont know if thats even less problematic to solve ...

again, this would not be a drop-in replacement and would require a rather big effort. One would have to assert that the respective helper methods are created (after the databinding block), and that they are created only once....

[col-panic]

Maybe we could leave databinding as is, and add these helper/compatibility methods to the main project? As long as those can then be used as drop-in replacement it would be fine.

[wimjongman]

One would have to assert that the respective helper methods are created (after the databinding block), and that they are created only once....

In addition, this requires that we have to rewrite the parsing and generation logic.

Maybe we could leave databinding as is, and add these helper/compatibility methods to the main project? As long as those can then be used as drop-in replacement it would be fine.

What do you mean by "main project"? Do you mean some kind of org.eclipse.jface.databinding.compatibilty package that serves as a drop-in replacement?

[laeubi]

Alright, given this is already some kind of Helper/Factory I don't see a reason not to provide such shorthand methods, as they don't come wit anny implementation/maintainance effort and should just be proper documented what they do.

[wimjongman]

Where should they go? In the new implementation, right?

[laeubi]

Where should they go? In the new implementation, right?

Yes i think so, seems the most suitable place.

[col-panic]

@laeubi are you planning to implement them - or what is the state here? 😕 If these are bound for the next release, then we might make a partial commit in eclipse-windowbuilder/windowbuilder#329 ?!

[laeubi]

@col-panic I think it would make sense for WB team to suggest an implementation for the parts required by WB, I think there is not much that needs intervention of committers here, so just propose a PR and I can review/merge if necessary. Actually @wimjongman is also commiter and can approve/merge as well. So if everything is fine, we should be able to get this published with the ibuild tomorrow.

[wimjongman]

@col-panic You mention in the windowbuilder issue this:

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

This is what we are aiming for. Can you analyze which classes we need to patch like this?

[col-panic]

I added a checklist in eclipse-windowbuilder/windowbuilder#329

On a first search I found the following

• org.eclipse.core.databinding.beans.PojoObservables
• org.eclipse.core.databinding.beans.BeansObservables
• org.eclipse.jface.databinding.swt.SWTObservable
• org.eclipse.jface.databinding.viewers.ViewersObservables

do you except those to return to the original plugins?

[wimjongman]

do you expect those to return to the original plugins?

No. The new classes must contain helper methods to ease the transition. Offer a drop-in replacement as you suggested.

[mickaelistria]

WindowBuilder should just copy, paste and maintain these classes in its own code. There was a consensus and a long review period and some alternatives documented to justify the removal. Now is IMO too late.

[wimjongman]

Yeah, that is not going to happen. We can support drop-in replacements. Otherwise, we most likely have to drop support for databinding.

WindowBuilder should just copy, paste and maintain these classes in its own code.

No thank you.

There was a consensus and a long review period and some alternatives documented to justify the removal.

and by "long review period" you mean that it was announced on some bugzilla and then you waited two years.

Platform did never consider consumers like WB. WB did not have a clue that such an established API was going to be dropped without a proper drop-in replacement. To be short, this was not properly analyzed.

Now is IMO too late.

There is always time to fix mistakes.

[col-panic]

do you expect those to return to the original plugins?

No. The new classes must contain helper methods to ease the transition. Offer a drop-in replacement as you suggested.

But where to provide these classes to the users then? You can't have a dependency on a windowbuilder plugin in your project. Would we have to create these classes as sources in the users bundles? Would we add a compatibility bundle?

[mickaelistria]

Platform did never consider consumers like WB. WB did not have a clue that such an established API was going to be dropped without a proper drop-in replacement. To be short, this was not properly analyzed.

Please see https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Fporting%2Fremovals.html&cp%3D2_3_0 ; the policy for deprecation is documented in https://wiki.eclipse.org/Eclipse/API_Central/Deprecation_Policy (now moved to https://github.com/eclipse-platform/.github/wiki/PMC-project-guidelines#api-removal-process ). The removal has happened according to this policy; was documented in code, and in help and linked from release notes for a while. Those classes have been publicly deprecated for enough time according to the policy; and no one has complained in time.

The failure for consumer projects to understand and embrace the Platform's policiy is an issue of consumers; not of providers.

WindowBuilder is the only project that requests those APIs back; it's not enough to consider re-introducing the burden in Platform. $PROJECT-specific issues can and should be fixed in $PROJECT.

To be short, this was not properly analyzed.

Platform is not responsible for every possible clients; particularly the dysfunctional ones (Window Builder was unfortunately unmaintained for a long time and missed to react accordingly). The analysis of Platform is correct, and if it shows that only 1 project does use an API, then it's a good thing this API is not worth being maintained by Platform.

[merks]

This also broke EMF's data binding example, but I didn't complain. 👅 Instead I made a copy of the class, because it's just an example and it's more work and impact for me to break the validation builds for older platforms than to just copy it into an example that probably no one cares about...

[laeubi]

The Generic variants where claimed to be a replacement for the "non generic" variant, so if there are methods missing that where present in the old class I think it is valid to add them back and it won't introduce any burden here as there is nothing to maintain.-

So I think if @col-panic can add missing methods I think this can be accepted. Maybe just start with the mentioned

SWTObservables.observeText(passText, SWT.Modify);
BeansObservables.observeValue(mySampleBean, "pass");

would be suitable and one can then further decide if there would be any problem (beside some class having a few more bytes that never change...)

[wimjongman]

All we are asking is a quick conversion from the old API to the new API.

Please see ...

I'm familiar with the rules of engagement, but since this discussion comes up every time Platform snips an API, it seems that it does not match reality.

Maybe the following rule should be added:

"The initiator of the API removal proposal should argue that the proposed deletion will have minimal impact on the Eclipse Ecosystem. From this argumentation, it should become clear that the API is not in widespread use and/or that the API can be replaced with minimal effort from the Ecosystem"

Because, when you think about it for a bit longer, dropping the complete existing databinding package is a huge, huge disruption.

Platform is not responsible ...
... embrace the Platform's policy is an issue of consumers; not of providers.

I get you, I do. At the same time, we should dial down the arrogance a bit. The biggest value of Eclipse lies in its current ecosystem and the applications built in the past.

What is Eclipse without WB, BIRT, EMF, Mylyn, XText, DBeaver, etc...? Applications are dropping, but I see no new ones. Platform cuts in its own flesh.

(Window Builder was unfortunately unmaintained for a long time and missed to react accordingly).

Not true. A small number of people are trying to keep the killer applications alive. We are just too few to keep up with Platforms' cleanup pace. This was recognized in the discussion we had on the mailing list. The conclusion of that discussion was that we would remove API only in M1 so that we have a chance to recover before release time.

and no one has complained in time

Because no one takes notice of the removal process. I expect the complaints to start coming when 2022-12 is released.

(At least one person woke up and exposed the risk, but this was dismissed with the same arguments. )

[col-panic]

The Generic variants where claimed to be a replacement for the "non generic" variant, so if there are methods missing that where present in the old class I think it is valid to add them back and it won't introduce any burden here as there is nothing to maintain.-

So I think if @col-panic can add missing methods I think this can be accepted. Maybe just start with the mentioned

SWTObservables.observeText(passText, SWT.Modify);
BeansObservables.observeValue(mySampleBean, "pass");

would be suitable and one can then further decide if there would be any problem (beside some class having a few more bytes that never change...)

To state this clear:

You want a Pull Request for https://github.com/eclipse-platform/eclipse.platform.ui that re-introduces

• org.eclipse.core.databinding.beans.BeansObservables
• org.eclipse.jface.databinding.swt.SWTObservables
• org.eclipse.core.databinding.beans.PojoObservables
• org.eclipse.jface.databinding.viewers.ViewersObservables
• ...

to the extent used by the code generated by the WindowBuilder.

[laeubi]

@col-panic no, but for example SWTObservables has (or had) the following hint:

Use WidgetProperties instead.

and you said you are missing SWTObservables.observeText then I think it is perfectly fine to add a method WidgetProperties#observeText that simply delegates to WidgetProperties.text().observe(..) ... if that helps WB and it wont add any "effort" as it just delegates.

[merks]

I agree with @laeubi I don't think you should reintroduce the deleted classes but rather add the "drop-in-replacement" helper methods to the replacement classes.

[col-panic]

@col-panic no, but for example SWTObservables has (or had) the following hint:

Use WidgetProperties instead.

and you said you are missing SWTObservables.observeText then I think it is perfectly fine to add a method WidgetProperties#observeText that simply delegates to WidgetProperties.text().observe(..) ... if that helps WB and it wont add any "effort" as it just delegates.

I'll try to realize this. Fun fact: If you search for e.g. "SWTObservables" in org.eclipse ... you still find references to a now non-existing class in some documentation parts

[col-panic]

Just an example, following up the "SWTObservable" path ... these are the classes in windowbuilder that start with the naming

but SWTObservable is now bound to merge into SWTProperties, where classes already exist ...

[wimjongman]

What do you mean by "bound to merge into SWTProperties"?

[col-panic]

What do you mean by "bound to merge into SWTProperties"?

That e.g. if it was SWTObservables.observeText(passText, SWT.Modify); up to now, it should now become SWTProperties.observeText(passText, SWT.Modify); according to #426 (comment) or isn't it?

[wimjongman]

Yes. Correct.