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

Reply via email to