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