Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if offset is valid before using it #367

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

FlorianKroiss
Copy link
Contributor

@FlorianKroiss FlorianKroiss commented Feb 7, 2024

The offset might be invalid when a user clicks on a location which is after the last character of a line.
The existing check no longer works, because getOffsetAtPoint only throws an IllegalArgumentException when the given Point is null.

… when a user clicks on a location which is after the last character of a line
@FlorianKroiss FlorianKroiss changed the title Check if offset is valid before using it. Check if offset is valid before using it Feb 7, 2024
@fabioz
Copy link
Owner

fabioz commented Feb 7, 2024

Maybe it could keep catching the exception (so that it keeps working the same on some older version of Eclipse -- i.e.: not sure when it stopped raising exception and started returning -1).

@fabioz
Copy link
Owner

fabioz commented Feb 7, 2024

p.s.: Thank you for the PR :)

@FlorianKroiss
Copy link
Contributor Author

FlorianKroiss commented Feb 7, 2024

The method did not change it's behavior. I rather think that this a regression from ba5f4f0, where the deprecated getOffsetAtLocation method (which threw the exception) was replaced with the recommended getOffsetAtPoint method which returns -1 in case of an invalid offset. So I think the new handling code should be fine and we don't need to keep the try-catch

@fabioz fabioz merged commit a1b4ed4 into fabioz:master Feb 7, 2024
1 check passed
@fabioz
Copy link
Owner

fabioz commented Feb 7, 2024

Great, thank you for taking the time to digg this up. Just merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants