On Tue, 30 May 2023 09:06:16 GMT, Alexey Ivanov <[email protected]> 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
