On Tue, 5 May 2026 13:29:47 GMT, Alexey Ivanov <[email protected]> wrote:
>> Phil Race has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - 8381843
>> - 8381843
>
> src/java.desktop/share/classes/java/awt/FileDialog.java line 455:
>
>> 453: * non-existent file. For example, it may be possible for the user
>> to
>> 454: * "accept" the result of a {@code setFile} value or to edit in the
>> name
>> 455: * of some other file to load/open even if it is not present.
>
> Will “…or to type a name…” be clearer?
>
> “…for _the_ `{@code LOAD}` mode…” — is the definite article missing?
>
> To me, the part after “to edit in” is still hard to understand.
No, it's not missing.
> src/java.desktop/share/classes/java/awt/FileDialog.java line 457:
>
>> 455: * of some other file to load/open even if it is not present.
>> 456: * Applications should therefore verify the file represented by the
>> 457: * {@code String} exists.
>
> “represented by the `{@code String}`” refers to the returned value. Should it
> be spelt out explicitly?
There's no other String involved here, so I don't see how there can be
ambiguity.
I can add the word "returned" but it doesn't seem necessary.
> src/java.desktop/share/classes/java/awt/FileDialog.java line 458:
>
>> 456: * Applications should therefore verify the file represented by the
>> 457: * {@code String} exists.
>> 458: * The value is usually the basename, not a full path name.
>
> Suggestion:
>
> * <p>
> * The value is usually the basename, not a full path name.
>
> Start a new paragraph?
I don't think that is necessary. If I were writing this as normal prose, I'd
consider it part of the same paragraph.
> src/java.desktop/share/classes/java/awt/FileDialog.java line 527:
>
>> 525: * use {@code setDirectory} to set the folder and {@code setFile} to
>> 526: * set a base file name.
>> 527: * <p>
>
> Start a new paragraph at “therefore”? It's an important detail, and it will
> stand out if the recommendation is in its own paragraph.
>
> “…to set ~~a~~ _the_ base file name.” — the definite article? It will align
> with “…to set _the_ folder…”.
I don't think it should be a new paragraph.
The use of "a" is deliberate. It implies more strongly that file should be "a
base name".
> src/java.desktop/share/classes/java/awt/FileDialog.java line 552:
>
>> 550: * Enables or disables multiple file selection for the file dialog.
>> 551: * <p>
>> 552: * Multiple mode may be ignored in some cases, for example commonly
>
> Suggestion:
>
> * Multiple mode may be ignored in some cases, for example, commonly
comma added
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30777#discussion_r3192043295
PR Review Comment: https://git.openjdk.org/jdk/pull/30777#discussion_r3192054135
PR Review Comment: https://git.openjdk.org/jdk/pull/30777#discussion_r3192060944
PR Review Comment: https://git.openjdk.org/jdk/pull/30777#discussion_r3192068926
PR Review Comment: https://git.openjdk.org/jdk/pull/30777#discussion_r3192082380