On Tue, 16 Jun 2026 21:13:15 GMT, Kevin Rushforth <[email protected]> wrote:

>> Andy Goryachev has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 194 commits:
>> 
>>  - test standard attributes
>>  - Merge branch 'master' into 8356042.ruler
>>  - ws
>>  - test doc props
>>  - cleanup
>>  - tab stops
>>  - Merge branch 'master' into 8356042.ruler
>>  - renamed doc
>>  - docs
>>  - Merge branch 'master' into 8356042.ruler
>>  - ... and 184 more: https://git.openjdk.org/jfx/compare/738be0f1...87241d12
>
> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/RichTextFormatHandler.java
>  line 544:
> 
>> 542:                         index += 2;
>> 543:                         // TODO check if already exists
>> 544:                         String ver = parseString();
> 
> In order for it to be effective, the version needs to be the first thing in 
> the file in addition to there being only one of them (and it eventually needs 
> to be mandatory). I'm still not sold on treating the version string as "just 
> another section", but at a minimum, you need to check whether this is the 
> first thing seen (not just the first version block), and fail if it isn't.

It is sort of byproduct of the serialization mechanism, in a sense that it 
emits a series of well-defined segments.  I suspect we might need to revisit 
the overall scheme once most of the requested attributes are in, and perhaps to 
change the way the model is being serialized altogether (for example, provide a 
more efficient binary format).

I'd like to leave this until later, with your permission.

> modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledTextModel.java
>  line 791:
> 
>> 789:             case DOCUMENT_PROPERTIES:
>> 790:                 handleDocumentProperties(seg.getDocumentProperties(), 
>> completeReplacement);
>> 791:                 break;
> 
> Doesn't this need to handle VERSION (in any case, this seems like another 
> reason not to treat version as just another attribute type).

(see my earlier response re:format)

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3424317271
PR Review Comment: https://git.openjdk.org/jfx/pull/1800#discussion_r3424320462

Reply via email to