-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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 bug related to multicursor and backspacing with brackets #89469
Fix bug related to multicursor and backspacing with brackets #89469
Conversation
scene/gui/code_edit.cpp
Outdated
int new_cc; | ||
if (_get_auto_brace_pair_close_at_pos(cl, cc) == idx) { | ||
remove_text(prev_line, prev_column, cl, cc + auto_brace_completion_pairs[idx].close_key.length()); | ||
new_cc = cc + auto_brace_completion_pairs[idx].close_key.length(); | ||
} else { | ||
remove_text(prev_line, prev_column, cl, cc); | ||
new_cc = cc; | ||
} |
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.
Could be simplified to:
int new_cc = cc;
if (_get_auto_brace_pair_close_at_pos(cl, cc) == idx) {
new_cc += auto_brace_completion_pairs[idx].close_key.length();
}
Note: Please make modifications by amending the commit, and force pushing the changes, so that it stays as a single commit (see PR workflow).
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.
Can also just use cc
instead of making new_cc
.
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.
cc
is still used further down in other branches and iterations of the loop, so modifying it might change behavior. Does it fix further bugs by doing so or actually risk introducing bugs?
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.
There is a continue
after this, and it is re-set on the next loop iteration, so it doesn't affect any other uses.
It won't fix any other bugs, its just cleaner.
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.
Ah indeed 👍
ae54a8a
to
769aff5
Compare
769aff5
to
4ab08fb
Compare
Thanks! And congrats for your first merged Godot contribution 🎉 |
Fixes #89417.
The root of the issue is that when removing an opening brace
remove_text
is sometimes called with justcc
(when the opening and closing brace are not next to each other), butadjust_carets_after_edit
was always being called assuming that the closing brace was removed as well.