On Tue, 14 Jul 2026 15:37:34 GMT, Andy Goryachev <[email protected]> wrote:
>> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/FileListFormatHandler.java
>> line 53:
>>
>>> 51: * Constructor.
>>> 52: */
>>> 53: public FileListFormatHandler() {
>>
>> This class is intended to be a singleton. Why does it have a public
>> constructor?
>
> singleton to allow reuse of the default implementation, public constructor to
> allow subclassing for custom models.
None of the other `DataFormatHandler` subclasses have a public constructor.
What makes this one different? How might a custom model override this?
>> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/FileListFormatHandler.java
>> line 121:
>>
>>> 119: return StyledSegment.of(" ", a);
>>> 120: }
>>> 121: } catch (Throwable e) {
>>
>> This should probably be be `catch (Exception e)` instead (we generally don't
>> catch `Error` in normal operation)
>
> The intent was to catch _anything_ - there is little value in bubbling
> anything up at this point.
_Anything_ usually doesn't include Error unless in a "run loop" (e.g., on the
renderer thread) or when calling a handler on the JavaFX application thread, or
similar, but we are not consistent on that at all in the JavaFX runtime, so I
won't object.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2196#discussion_r3581236731
PR Review Comment: https://git.openjdk.org/jfx/pull/2196#discussion_r3581247690