Hi, Maybe it could generate the boilerplate like Lombok does? So it has the benefits of eliminating error-prone boilerplate code and the benefits of not having the cost of reflection at runtime.
-- Thiago. Em qua., 6 de dez. de 2023 às 10:28, John Hendrikx <[email protected]> escreveu: > I also like this route, perhaps slightly more, depending on how far this > can be taken: > > Disclaimer: I dislike the CSSMetaData and special property subclass a lot > -- it's a major concession to make properties work with the CSS system that > brings a lot of boilerplate that is mostly unnecessary. > > If you go the route of an annotation, we could offer to do everything via > the annotation so a regular property can be converted to styleable by > annotating it. The property would then have to specify the name > ("-fx-label-padding") and the default value (which can be a string that is > parseable via the existing CSS parser). > > In other words: > > @Styleable(name = "-fx-label-padding", defaultValue = "(0, 0, 0, 0)") > public final ReadOnlyObjectProperty<Insets> labelPaddingProperty() { ... } > The rest the scanner can figure out (it could even figure out a default > name). The `isSettable` method is always the same boilerplate (and if not, > you can use the old way). The required converter can be derived from the > generic type (Insets -> InsetsConverter), or specified (converter = > InsetsConverter.class). > > To convert a regular property into a styleable does require somewhat more > magic (the CSS system would have to wrap it, or otherwise track them) but > that's all hidden. It gets rid of the StyleableProperty uglyness and the > need for users to create those, and puts the "extra" CSSMetaData > information a styleable property needs firmly where it belongs (in an > annotation). > > --John > > On 06/12/2023 11:37, Nir Lisker wrote: > > I thought about the option of reflection, but I opted to propose > annotations instead. The following is my reasoning. > > Firstly, reflection is very magic-y. The author of the class has no > indication of what side effects happen due to the code they write, the > output (css handling in this case) comes out of nowhere from their > perspective. As with other reflection cases, it is a "pull" rather than > "push" approach - you don't write what should happen, you let someone else > decide that. For writers of skin/control classes, this means that they need > to know exactly what constitutes a hook for the reflection mechanism, or > face surprises. There is no compile time check that tells you whether you > have declared your styleable property properly or not (without an external > ad-hoc checker). > We do this somewhat with properties - any method of the form > "...property()" gets special treatment, but this is for the docs. I don't > think we have code that depends on this other than in tests. > > Secondly, the proposed mechanism depends on the runtime type, not the > declared type. As a user, I see no indication in the API whether a property > is styleable or not. This is also (what I would consider) a problem with > the current state. When I thought about using reflection to solve this, I > at least thought to specify the declared type of the property as styleable, > like StyleableBooleanProperty instead of BooleanProperty (restricting the > returned type is backwards compatible). A downside of this is that it gives > access to the methods of StyleableProperty, which are not useful for the > user, I think, but maybe someone has a use for them. > > Thirdly, maybe I want to declare a styleable property not to be used > automatically. I can't think off the top of my head when I would want to do > that, but I'm also not a heavy css user. Are we sure that just initializing > a property with a styleable runtime type should *always* be caught by this > process? > > To compare, annotations have the following benefits: > > Firstly, they are declarative, which means no surprises for the class > author (WYSIWYG). This also allows more flexibility/control over which > properties get special treatment via an opt-in mechanism. > > Secondly, They can also be displayed in the JavaDocs (via @Documented) > with their assigned values. For example, the padding property of Region can > be annotated with @Styleable(property="-fx-padding"), informing the user > both that this value can be set by css, and how to do it. Interestingly, > the annotation doesn't need to be public API to be displayed, so we are not > bound by contracts. > > In terms of similarities: > > In both the reflection and the annotation proposals, the steps are: > 1. Create styleable properties. > 2. That's it. > It's just that step 1 also adds an annotation to the creation of the > property (which was/is a 2-step process anyway, declaring the property and > its css metadata). > > Annotations also require a processor to read the data from their values > and target (the field/method). This is a bit of work, but > Michael's CssMetaDataCache class is basically that - read the data from the > class (via reflection or annotations) and store it in a map. The logic > should be the same, just the method to obtain the data is different. Both, > as a result, have the benefits of handling control/skin combinations (what > I mentioned in the point "Usable both in controls and in skins (or other > classes)"). > > The benefit of co-locating the property and its css metadata in the class > itself also remains. > > > To summarize, both approaches eliminate all the clutter of writing > styleable properties (John, will you like to create styleable properties > now? [1] :) ), both apply the flexibility of caching per class, both allow > better structuring of the class, but they read the properties differently > and have a different level of declarativness. > > [1] > https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044010.html > > > On Tue, Dec 5, 2023 at 11:21 PM Andy Goryachev <[email protected]> > wrote: > >> I like the idea. >> >> >> >> I wonder if it is possible to reduce the amount of boilerplate code? For >> example, a CssMetaData can have a >> >> >> >> setGetter(Function<S, StyleableProperty<V>> getter) >> >> >> >> method which supplies the property reference? This way >> CssMetaData.isSettable(Node) and CssMetaData.getStyleableProperty(Node) can >> be implemented in the base class (there are more complicated cases, so >> perhaps setIsSettable(Predicate<Node>) would also be required). >> >> >> >> Example: >> >> >> >> CssMetaData.<ControlExample,Font>of("-fx-font", Font.getDefault(), (n) >> -> n.font) >> >> >> >> Just a thought. What do you think? >> >> >> >> -andy >> >> >> >> >> >> *From: *openjfx-dev <[email protected]> on behalf of Michael >> Strauß <[email protected]> >> *Date: *Sunday, December 3, 2023 at 22:02 >> *To: *openjfx-dev <[email protected]> >> *Subject: *Reflective discovery of styleable properties >> >> Following up the discussion around the CssMetaData API, I'd like to >> chime in with yet another idea. To recap, here's Nir's summary of the >> current API [0]: >> >> "Let's look at what implementation is required from a user who wants >> to write their own styleable control: >> 1. Create styleable properties. >> 2. Create a list of these properties to be passed on. >> 3. Create a public static method that returns the concatenation of >> this list with the one of its parent. (This method happens to be >> poorly documented, as mstr said.) >> 4. Create a public non-static method that calls the static method in a >> forced-override pattern because otherwise you will be calling the >> wrong static method. (This method's docs seem to be just wrong because >> you don't always want to delegate to Node's list.)" >> >> >> I think this could reasonably be replaced with the following >> implementation requirements: >> 1. Create styleable properties. >> 2. That's it. >> >> Let's look at what we're actually trying to do: create a list of >> CSS-styleable property metadata of a class. But we can easily do that >> without all of the boilerplate code. >> >> When ´Node.getCssMetaData()` is invoked, all public methods of the >> class are reflectively enumerated, and metadata is retrieved from >> `Property` and `StyleableProperty` getters. This is a price that's >> only paid once for any particular class (i.e. not for every instance). >> The resulting metadata list is cached and reused for all instances of >> that particular class. >> >> As a further optimization, metadata lists are also cached and >> deduplicated for Control/Skin combinations (currently every Control >> instance has its own copy of the metadata list). >> >> Another benefit of this approach is that the CssMetaData can now be >> co-located with the property implementation, and not be kept around in >> other parts of the source code file. Here's how that looks like when a >> new "myValue" property is added to MyClass: >> >> StyleableDoubleProperty myValue = >> new SimpleStyleableDoubleProperty(this, "myValue") { >> >> static final CssMetaData<MyClass, Number> METADATA = >> new CssMetaData<MyClass, Number>( >> "-fx-my-value", >> SizeConverter.getInstance(), >> USE_COMPUTED_SIZE) { >> @Override >> public boolean isSettable(MyClass node) { >> return !node.myValue.isBound(); >> } >> >> @Override >> public StyleableProperty getStyleableProperty( >> MyClass node) { >> return node.myValue; >> } >> }; >> >> @Override >> public CssMetaData getCssMetaData() { >> return METADATA; >> } >> }; >> >> public final DoubleProperty myValueProperty() { >> return myValue; >> } >> >> It is not required to override the `getCssMetaData()` method, nor is >> it required to redeclare a new static `getClassCssMetaData()` method. >> It is also not required to manually keep the list of styleable >> properties in sync with the list of CSS metadata. >> >> I've prototyped this concept for the `Node`, `Region` and `Control` >> classes [1]. >> >> [0] >> https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044046.html >> [1] https://github.com/openjdk/jfx/pull/1299 >> >
