RFR: 8301302: Platform preferences API [v30]

[Michael Strauß]

On Fri, 24 Nov 2023 11:41:30 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   flip S and T
>
> modules/javafx.graphics/src/main/java/com/sun/glass/ui/Application.java line 770:
> 
>> 768: 
>> 769:     /**
>> 770:      * Returns a map of platform-specific preference keys to well-known keys.
> 
> Not a fan of the wording "well-known keys".  They're platform independent keys as JavaFX defines them. Suggestions "JavaFX keys", "JavaFX standard keys", "FX keys, which are platform independent keys defined by JavaFX".

I changed it to the wording "platform-independent keys".

> modules/javafx.graphics/src/main/java/com/sun/glass/ui/Application.java line 793:
> 
>> 791:      * Returns a mapping of platform-specific keys to the types of their values.
>> 792:      * Polymorphic types are supported by specifying the common base type; for example, a key can
>> 793:      * be mapped to {@code Paint.class} to support any type of paint.
> 
> May want to search for mentions of `Paint` now that its removed, and use a different example here.

I think the example is probably one of the most practically relevant use cases, even though right now there aren't any mappings with a declared type of `Paint`.

> modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/GtkApplication.java line 476:
> 
>> 474:     @Override
>> 475:     public Map<String, String> getPlatformKeyMappings() {
>> 476:         return Map.of(
> 
> IMHO this should be returned from a `private static final`

I don't understand. This is an overridden method, do you propose to introduce another `private static final` method, and `getPlatformKeyMappings` then calls out to this other method?

> modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 314:
> 
>> 312: 
>> 313:     // Assuming S is a class type:
>> 314:     private boolean isClassConvertible(Class<?> S, Class<?> T) {
> 
> Locals here don't follow the Java naming conventions, which explains why the code is somewhat confusing to read.
> 
> Same for the other functions.

Changed.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1404572373
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1404573061
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1404570744
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1404572002