On Fri, 19 Jan 2024 10:21:05 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Moves `SimpleSelector` and `CompoundSelector` to internal packages. >> >> This can be done with only a minor API break, as `SimpleSelector` and >> `CompoundSelector` were public before. However, these classes could not be >> constructed by 3rd parties. The only way to access them was by doing a cast >> (generally they're accessed via `Selector` not by their sub types). The >> reason they were public at all was because the CSS engine needs to be able >> to access them from internal packages. >> >> This change fixes a mistake (or possibly something that couldn't be modelled >> at the time) when the CSS API was first made public. The intention was >> always to have a `Selector` interface/abstract class, with private >> implementations (`SimpleSelector` and `CompoundSelector`). >> >> This PR as said has a small API break. The other changes are (AFAICS) >> source and binary compatible: >> >> - Made `Selector` `sealed` only permitting `SimpleSelector` and >> `CompoundSelector` -- as `Selector` had a package private constructor, there >> are no concerns with pre-existing subclasses >> - `Selector` has a few more methods that are now `protected` -- given that >> the class is now sealed, these modified methods are not accessible (they may >> still require rudimentary documentation I suppose) >> - `Selector` now has a `public` default constructor -- as the class is >> sealed, it is inaccessible >> - `SimpleSelector` and `CompoundSelector` have a few more `public` methods, >> but they're internal now, so it is irrelevant >> - `createMatch` was implemented directly in `Selector` to avoid having to >> expose package private fields in `Match` for use by `CompoundSelector` >> - No need anymore for the `SimpleSelectorShim` > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Add since tags Hmm. This suggests taking a step back. I think there are only two choices that make sense (there are a couple others, but I don't think they are useful). 1. Abandon the notion of making an incompatible change to `SimpleSelector` and `CompoundSelector`. This means that PR #1316 will need to preserve the existing `SimpleSelector` method `List<String> getStyleClasses()` and `Set<StyleClass> getStyleClassSet()`, deprecating them with simple deprecation (not for removal). 2. Make an incompatible change, meaning that at some point, the existing SceneBuilder binaries will break. We could make the transition a little less painful by providing replacement API in a release that still has the existing API (in fact, I think if we go with this option, then we _must_ do that), but the fact remains that there will then be no way to have a single binary that runs on JavaFX 21 and JavaFX 23 short of a multi-release jar file, which really isn't a good solution for this sort of problem. Preserving the existing `SimpleSelector` methods is fairly easy and already done. So, is there any real drawback to doing this? I can't think of any. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1333#issuecomment-1900861894
