<div dir="ltr">Hi,<div><br></div><div>Maybe it could generate the boilerplate like Lombok does?</div><div>So it has the benefits of eliminating error-prone boilerplate code and the benefits of not having the cost of reflection at runtime.<br></div><div><br></div><div>-- Thiago.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Em qua., 6 de dez. de 2023 às 10:28, John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank">john.hendrikx@gmail.com</a>> escreveu:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p>I also like this route, perhaps slightly more, depending on how
far this can be taken:<br>
</p>
<p>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.</p>
<p>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).</p>
<p>In other words:</p>
<p><span style="background-color:rgb(255,255,255);padding:0px 0px 0px 2px"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:Consolas;font-size:11pt;white-space:pre-wrap"><span style="color:rgb(0,0,0)"></span><span style="color:rgb(0,0,160);font-weight:bold"> @Styleable(name = "-fx-label-padding", defaultValue = "(0, 0, 0, 0)")
public</span><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">final</span><span style="color:rgb(0,0,0)"> ReadOnlyObjectProperty<Insets> labelPaddingProperty() { ... }</span></span></span></p>
<div>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).</div>
<div><br>
</div>
<div>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).</div>
<br>
<div>--John<br>
</div>
<div><br>
</div>
<div>On 06/12/2023 11:37, Nir Lisker wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">I thought about the option of reflection, but I
opted to propose annotations instead. The following is my
reasoning.
<div><br>
</div>
<div>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).</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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?</div>
<div><br>
</div>
<div>To compare, annotations have the following benefits:</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.<br>
</div>
<div><br>
</div>
<div>In terms of similarities:</div>
<div><br>
</div>
<div>In both the reflection and the annotation proposals, the
steps are:</div>
1. Create styleable properties.<br>
2. That's it.
<div>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).<br>
</div>
<div><br>
</div>
<div>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 "<span style="color:rgb(232,230,227)">Usable
both in controls and in skins (or other classes)")</span>.</div>
<div><br>
</div>
<div>The benefit of co-locating the property and its css
metadata in the class itself also remains.</div>
<div><br>
</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>[1] <a href="https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044010.html" target="_blank">https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044010.html</a></div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Dec 5, 2023 at
11:21 PM Andy Goryachev <<a href="mailto:andy.goryachev@oracle.com" target="_blank">andy.goryachev@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<div lang="EN-US">
<div>
<p class="MsoNormal"><span>I like the idea.</span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal"><span>I wonder if it is possible to
reduce the amount of boilerplate code? For example,
a CssMetaData can have a</span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal" style="text-indent:0.5in"><span>setGetter(Function<S,
StyleableProperty<V>> getter)</span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal"><span>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).</span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal"><span>Example:</span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal" style="background:rgb(232,242,254)"><span lang="FR">CssMetaData.<ControlExample,Font>of(</span><span lang="FR">"-fx-font"</span><span lang="FR">,
Font.getDefault(), (n) -> n.font)</span></p>
<p class="MsoNormal"><span lang="FR"> </span></p>
<p class="MsoNormal"><span>Just a thought. What do you
think?</span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal"><span>-andy</span></p>
<p class="MsoNormal"><span> </span></p>
<p class="MsoNormal"><span> </span></p>
<div id="m_6823917702071497988m_-5766188174560428208m_-7571650920342053996m_640328469919311222m_-4650580218595929338mail-editor-reference-message-container">
<div>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt solid rgb(181,196,223);padding:3pt 0in 0in">
<p class="MsoNormal" style="margin-bottom:12pt"><b><span style="font-size:12pt;color:black">From:
</span></b><span style="font-size:12pt;color:black">openjfx-dev
<<a href="mailto:openjfx-dev-retn@openjdk.org" target="_blank">openjfx-dev-retn@openjdk.org</a>>
on behalf of Michael Strauß <<a href="mailto:michaelstrau2@gmail.com" target="_blank">michaelstrau2@gmail.com</a>><br>
<b>Date: </b>Sunday, December 3, 2023 at
22:02<br>
<b>To: </b>openjfx-dev <<a href="mailto:openjfx-dev@openjdk.org" target="_blank">openjfx-dev@openjdk.org</a>><br>
<b>Subject: </b>Reflective discovery of
styleable properties</span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11pt">Following
up the discussion around the CssMetaData API,
I'd like to<br>
chime in with yet another idea. To recap,
here's Nir's summary of the<br>
current API [0]:<br>
<br>
"Let's look at what implementation is required
from a user who wants<br>
to write their own styleable control:<br>
1. Create styleable properties.<br>
2. Create a list of these properties to be
passed on.<br>
3. Create a public static method that returns
the concatenation of<br>
this list with the one of its parent. (This
method happens to be<br>
poorly documented, as mstr said.)<br>
4. Create a public non-static method that
calls the static method in a<br>
forced-override pattern because otherwise you
will be calling the<br>
wrong static method. (This method's docs seem
to be just wrong because<br>
you don't always want to delegate to Node's
list.)"<br>
<br>
<br>
I think this could reasonably be replaced with
the following<br>
implementation requirements:<br>
1. Create styleable properties.<br>
2. That's it.<br>
<br>
Let's look at what we're actually trying to
do: create a list of<br>
CSS-styleable property metadata of a class.
But we can easily do that<br>
without all of the boilerplate code.<br>
<br>
When ´Node.getCssMetaData()` is invoked, all
public methods of the<br>
class are reflectively enumerated, and
metadata is retrieved from<br>
`Property` and `StyleableProperty` getters.
This is a price that's<br>
only paid once for any particular class (i.e.
not for every instance).<br>
The resulting metadata list is cached and
reused for all instances of<br>
that particular class.<br>
<br>
As a further optimization, metadata lists are
also cached and<br>
deduplicated for Control/Skin combinations
(currently every Control<br>
instance has its own copy of the metadata
list).<br>
<br>
Another benefit of this approach is that the
CssMetaData can now be<br>
co-located with the property implementation,
and not be kept around in<br>
other parts of the source code file. Here's
how that looks like when a<br>
new "myValue" property is added to MyClass:<br>
<br>
StyleableDoubleProperty myValue =<br>
new
SimpleStyleableDoubleProperty(this, "myValue")
{<br>
<br>
static final CssMetaData<MyClass,
Number> METADATA =<br>
new CssMetaData<MyClass,
Number>(<br>
"-fx-my-value",<br>
SizeConverter.getInstance(),<br>
USE_COMPUTED_SIZE) {<br>
@Override<br>
public boolean isSettable(MyClass
node) {<br>
return
!node.myValue.isBound();<br>
}<br>
<br>
@Override<br>
public StyleableProperty
getStyleableProperty(<br>
MyClass node) {<br>
return node.myValue;<br>
}<br>
};<br>
<br>
@Override<br>
public CssMetaData getCssMetaData() {<br>
return METADATA;<br>
}<br>
};<br>
<br>
public final DoubleProperty
myValueProperty() {<br>
return myValue;<br>
}<br>
<br>
It is not required to override the
`getCssMetaData()` method, nor is<br>
it required to redeclare a new static
`getClassCssMetaData()` method.<br>
It is also not required to manually keep the
list of styleable<br>
properties in sync with the list of CSS
metadata.<br>
<br>
I've prototyped this concept for the `Node`,
`Region` and `Control` classes [1].<br>
<br>
[0] <a href="https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044046.html" target="_blank">
https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044046.html</a><br>
[1] <a href="https://github.com/openjdk/jfx/pull/1299" target="_blank">https://github.com/openjdk/jfx/pull/1299</a></span></p>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
</blockquote></div>