-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
Fix TextEdit column index out of bound if column length changes during selection #83451
Fix TextEdit column index out of bound if column length changes during selection #83451
Conversation
06c2e18
to
ceeadf9
Compare
@Lunarisnia So cool 👌 |
ceeadf9
to
f414734
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice investigation!
TextEdit
changes the selection directly in set_line
which probably causes this inconstancy in the first place, so we probably need to add another check there for .selection.selecting_column
, otherwise might find this error occurring in a few other places.
f414734
to
8c0220e
Compare
I've moved the fix into |
@@ -3445,6 +3445,7 @@ void TextEdit::set_line(int p_line, const String &p_new_text) { | |||
|
|||
if (has_selection(i) && p_line == get_selection_to_line(i) && get_selection_to_column(i) > text[p_line].length()) { | |||
carets.write[i].selection.to_column = text[p_line].length(); | |||
carets.write[i].selection.selecting_column = CLAMP(carets[i].selection.selecting_column, 0, text[p_line].length()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would add a new check for selecting_column
as it's not guaranteed that get_selection_to_line() == selecting_line
@Lunarisnia Are you available to address latest feedback? |
@YuriSizov Sorry for letting this go stale, I have been quite busy these few weeks, I don't think I can address the feedback this week. Maybe I'll just close this so that people can take over? |
@Lunarisnia There is no rush if you plan to eventually finish it! But voicing that others are welcome to take over is also fine. If anyone pick it up, we can always supersede your PR. In any case, no worries. |
Superseded by #86978. Thanks for the contribution! |
Fixes #83410