On Mon, 29 Apr 2024 12:35:19 GMT, Ambarish Rapte <[email protected]> wrote:
>> Oliver Kopp has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Revert using utility method
>
> modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinTextRangeProvider.java
> line 124:
>
>> 122: // In case there was an overflow, return the maximum end index
>> 123: if (res < 0) return maxEndIndex;
>> 124: return res;
>
> In this class,
> - The index **start** and **end** are guaranteed to be +ve values such that,
> `0 <= start <= end <= length` (length is the total length of text in the text
> component). : please check the method `validateRange()`
> - So we only need to validate that the variable `length` in this helper
> method is valid i.e.
> 1. `length > 0` and
> 2. `length <= end - start`
>
>
> /**
> * In the context of substrings, this is a helper method to get the end
> offset index
> * from the start index for a given length of a substring.
> *
> * @param startIndex The start index of the range in the string. Needs to
> be 0 or more (not checked in the code).
> * @param length The requested length of a substring in the range when
> starting from "start".
> * Invalid values are treated as full length.
> * @param endIndex The end index of the range in the string. Needs to be
> equal or greater than startIndex
> * (not checked in the code).
> */
> static int getEndOffsetIndex(int startIndex, int length, int endIndex) {
> if (length < 0 || (endIndex - startIndex) <= length) {
> return endIndex;
> }
> return startIndex + length;
> }
>
> - With this change
> 1. The start and end index values are always positive and hence negative
> value tests should be removed.
> 2. The name of method is changed so, the Shim class would need a change
> too.
> 3. The fix could be as simple as below one line correction in the ternary
> statement in the GetText() method, but then we won't be able to add the test.
> So keeping the helper method would be good idea.
> `int endOffset = (length < 0 || (endIndex - startIndex) <= length) ? end :
> start + maxLength;`
Smalltalk: I could go back to my first commit
https://github.com/openjdk/jfx/commit/e81712ca12035e6607c9a31379fa0180be8be7fd
to be consistent with the other style in the class 🤣🤣🤣. Only joking, the new
code is more readable to Java newcomers IMHO. 😅✌️
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1583344489
