On Fri, 19 Apr 2024 18:32:06 GMT, Oliver Kopp <d...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinTextRangeProvider.java >> line 378: >> >>> 376: if (text == null) return null; >>> 377: validateRange(text); >>> 378: int endOffset = getValidStringIndex(start, maxLength, end); >> >> If I presume the arguments are (start, length, maxLength), the last two >> arguments on this line only need to be swapped. They seem to be correct on >> LL 394, 498. > > `maxLength` is the parameter of the `GetText` method. The caller asks for the > text, but only with `maxLength` length. Therefore, `start + maxLength`. `end` > is the real end of the current string. After `end` there are no characters. > Ensured by `validateRange`. > > I can add a comment. I love to add comments, but I learned at > https://github.com/openjdk/jdk/pull/10704 that only higher-level comments are > desired... I think the comment in https://github.com/openjdk/jdk/pull/10704 referred to an external project, so it was deemed as unrelated. but this method, I feel, is rather convoluted. why do we need to catch exceptions (malloc!) when we should be able to implement the required functionality with a few `if`s. So I really have two comments: a) the code looks way too complex and b) if an average developer (like me) cannot figure out what it does within 10 seconds, a good comment would help. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1572773987
