RFR: 8307105: JFileChooser InvalidPathException when selecting some system folders on Windows [v7]

Tejesh R tr at openjdk.org
Wed May 31 06:16:02 UTC 2023


On Tue, 30 May 2023 09:06:16 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Tejesh R has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
>> 
>>  - Updated new fix
>>  - Merge branch 'master' of https://git.openjdk.java.net/jdk into branch_8307105
>>  - White space fix
>>  - Alternate Fix
>>  - Updated based on review comments
>>  - Updated based on review comments
>>  - Updated based on review comments
>>  - Fix + Manual Test
>
> src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 867:
> 
>> 865:             cachedIsLink = hasAttribute(ATTRIB_LINK)
>> 866:                     && (!isFileSystem()
>> 867:                     || getPath().toLowerCase().endsWith(".lnk"));
> 
> At the very least, you should indent `||` farther because it's inside the parentheses (the condition is `c1 && (!c2 || c3)`), otherwise it could be misinterpreted as `c1 && !c2 || c3` which is completely different.
> Suggestion:
> 
>             cachedIsLink = hasAttribute(ATTRIB_LINK)
>                     && (!isFileSystem()
>                         || getPath().toLowerCase().endsWith(".lnk"));
> 
> 
> However, my personal preference is to align wrapped lines to the start of their corresponding part of the expression:
> Suggestion:
> 
>             cachedIsLink = hasAttribute(ATTRIB_LINK)
>                            && (!isFileSystem()
>                                || getPath().toLowerCase().endsWith(".lnk"));
> 
> 
> Either way is fine.

Updated to second suggestion.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13998#discussion_r1211134303



More information about the client-libs-dev mailing list