Overhaul multicaret editing and selection in TextEdit

[kitbdev]

• related #73471
• fixes #86863 (all issues except reload script and goto line)
• fixes #83410
• fixes #72797
• probably fixes #74400 (can't reproduce, related to other issues fixed)
• fixes #73202
• fixes #81535
• fixes #83826
• fixes https://github.com/godotengine/godot/issues/80378
• fixes #83589
• fixes https://github.com/godotengine/godot/issues/90462
• supersedes (can still cherry-pick) #84901
• supersedes (can still cherry-pick) #81331
• supersedes #83451

Selection now goes from selection origin to the caret, so that the caret is always attached to the selection.

All TextEdit APIs now handle multiple carets. This should make it easier to use and use correctly. If a method is called alone, it will merge overlapping carets. If it is called in a loop for multiple carets as part of a multicaret edit, the merge will happen afterwards to not affect caret indexes.

All text changes and caret changes now queue_redraw().

Implementation details

General

• Added remove_line_at() and insert_text() in TextEdit to flesh out the API because methods now affects carets in different ways, so the way it is used and the context matters.
  • For example: set_line() was often used to insert text, but this doesn't allow for much context of how the carets on that line should be affected.
• Moved move_lines_up/down(), delete_lines(), duplicate_selection(), and delete_lines() from CodeEditor to CodeEdit since the implementations changed so they can be tested, and based on this https://github.com/godotengine/godot/pull/66553#pullrequestreview-1598392668.
• Made drag and drop text use a new caret instead of the main one so it won't interfere with any carets.
  • get_caret_count() and many other methods now ignores the drag caret if it exists, as it shouldn't be used as a regular caret in most cases.
  • I wanted it to not be part of the carets vector at all, but I also didn't want to copy lots of drawing and scrolling code.
• Added _text_changed(), _caret_changed(), and _selection_changed() to have one clear place after a change that will be called immediately. These are used to redraw, defer emit signals, and cancel drag and drop if needed.
  • A selection_changed signal should be easy to add now, if it is needed in the future.
• Added _cancel_drag_and_drop_text() to cancel the Viewport's drag and drop in certain circumstances.
  • Added Viewport::gui_cancel_drag() to support this, since I couldn't find an existing way of cancelling a drag and drop operation.
• Added _unhide_carets() because sometimes the TextEdit needs to unfold a line, and it can't just use unhide the line.
• Changed _cut_internal() to use _copy_internal() then just remove the selection/lines as it is much simpler.
• In set_caret_line() the p_wrap_index param now accepts -1 to only clamp the column and not try to adjust it. In many cases set_caret_column() is called immediately afterward, so calculating the column so it lines up visually was unnecessary, and sometimes needed a workaround to avoid.
• Changed some MessageQueue calls to use callables, since I moved them around. (related #86301)
• Added cancel_ime() and apply_ime() for more control over the IME.
  • Added internally _close_ime_window(), _update_ime_window_position(), and _update_ime_text() to reduce code duplication and increase clarity.
  • We may want to make similar changes in LineEdit?
• Added get/set_carets_state() for the various reload_text() methods and for getting/setting the entire state.
• Updated some comments style. I didn't change the block comments that are used as headers, since they should probably be distinct from regular comments and I didn't know what style to change them to.
• Changed GotoLineDialog::popup_find_line() to use a CodeTextEditor instead of a CodeEdit so it can use the existing goto_line() functionality and not need to recreate it.
  • GotoLineDialog should use a SpinBox or something, but that can be a separate PR.

TextEdit Multicaret

• Added begin/end_multicaret_edit() to be able to allow merging the carets when the edit is complete.
  • These should always come in pairs.
  • These can be nested, and is what allows for more complex editing to use regular APIs, and have both merge only once at the end of the edit.
• Added queue_merge_carets() to be used when merging should happen at the end of the multicaret edit.
• Added multicaret_edit_ignore_caret() that should be used when iterating over carets when editing them and text is removed. This will return true when the given caret would have been removed due to it merging with another caret, or it was just added.
  • This allows multiple carets to be collapsed into one and delay the merging without affecting the edit.
• Added collapse_carets() to move all carets in a region of deleted text to one overlapping location and add to the ignore list. These will be merged at the end of the edit unless they are separated.
• Removed get_caret_index_edit_order(), because it is no longer needed since carets can be edited in any order.
• Added get_sorted_carets() to replace some functionality from get_caret_index_edit_order, but in the opposite order. It sorts by selection_from_line instead of selection_to_line, in ascending order (top of page to bottom).
• get_sorted_carets() is const and doesn't cache because it is usually only needed after a caret is moved which would invalidate the cache. I also use a Comparator so it should be faster, O(log(n)) instead of O(n^2). If there are still performance concerns, we can change it to use a cache.
• Added get_line_ranges_from_carets() to get a vector of all the lines that are part of a selection or have a caret on them. This is useful for many common editing tasks.
  • This was inspired by the removed methods in CodeTextEditor _get_affected_lines_from/to(), but more comprehensive.
• Removed adjust_carets_after_edit() because it is no longer needed now that carets are automatically handled.
  • _offset_carets_after() is similar (though not the same) and is used internally for a similar purpose, to move all carets after some text was added/removed.
• Updated all code that operates on multiple carets.
  • TextEdit::merge_overlapping_carets(), TextEdit::_new_line(), TextEdit::_do_backspace(), TextEdit::_delete(), TextEdit::insert_text_at_caret(), TextEdit::add_caret_at_carets(), TextEdit::adjust_carets_after_edit(), TextEdit::get_selected_text(), TextEdit::delete_selection(), TextEdit::_handle_unicode_input_internal(), TextEdit::_backspace_internal(), TextEdit::_cut_internal(), TextEdit::_copy_internal(), TextEdit::_paste_internal(), CodeEdit::_handle_unicode_input_internal(), CodeEdit::_backspace_internal(), CodeEdit::do_indent(), CodeEdit::indent_lines(), CodeEdit::unindent_lines(), CodeEdit::_new_line(), CodeEdit::create_code_region(), CodeEdit::confirm_code_completion(), CodeEdit::duplicate_lines(), TextEditor::_edit_option(EDIT_TOGGLE_FOLD_LINE), ScriptTextEditor::_edit_option(EDIT_TOGGLE_FOLD_LINE), ScriptTextEditor::_edit_option(DEBUG_TOGGLE_BREAKPOINT), CodeTextEditor::convert_case(), CodeTextEditor::move_lines_up(), CodeTextEditor::move_lines_down(), CodeTextEditor::delete_lines(), CodeTextEditor::duplicate_selection(), CodeTextEditor::toggle_inline_comment(), CodeTextEditor::toggle_bookmark()

TextEdit Selection

• Changed select() to now select from a 'selection origin' to the caret. Calling it will move the caret.
• get_selection_from/to_line/column still gives the start/end of the selection and can be used like before. Since these values are now calculated and its a common use case, they return the caret line/column if there is no active selection.
• Added set_selection_origin_line/column() to be able to set the origin of the selection directly, similar to set_caret_line/column().
  • Added a get_selection_origin_line/column(). Deprecated get_selection_line/column() because the functionality is a bit different, and I wanted a more clear name. It didn't always return the origin of the selection, it was just used to setup the different selection modes.
  • These functions work even if there is no selection because it is sometimes needed to use or set it before starting a selection, like in _pre_shift_selection() or add_caret_at_carets().
• Selections can now touch each other since when I was reworking on merge_overlapping_carets(), I used VSCode for reference and that is how it is done there. It is also useful in some scenarios.
• Changed has_selection(), get_selected_text(), and deselect() to be faster O(1) if the caret index is passed in instead of O(n). I didn't do this for all of them since there would be lots of copy pasted code.
• Added is_selection_direction_right() to be able to tell if the selection is left to right (top to bottom) or the reverse, since it now always has a direction.
• Removed _post_shift_selection() as it wasn't needed anymore.
• This comment in _click_selection_held() is no longer true:
  • Warning: is_mouse_button_pressed(MouseButton::LEFT) returns false for double+ clicks...
  • I'm not sure when this changed but it does work for word and line mode.
• Added selection_contains(), get_selection_at(), and is_line_col_in_range() as helper functions to reduce duplicate code.

Tests

• Added clipboard and primary clipboard support to DisplayServerMock to be able to test cut, copy, paste, and paste primary
  • Removed those todos
• Added build_array() to TestTextEdit and TestCodeEdit to be able to easily create arrays, copied from variant/test_array.h.
  • These can be replaced with initializer lists after #86015.
• Updated comment style for CodeEditTests.
• Added SEND_GUI_KEY_UP_EVENT test macro, for some text drag tests where the key up event was needed.
• Added test cases for insert text, remove text, remove line at
• Improved/added tests in areas that had bugs or where I changed lots of code.
• Removed todo // Add undo / redo tests? line 1368 by adding undo redo tests for each input subcase.
• Lots of lines_edited_args had to be changed since the order edits are done in are different, but this shouldn't be an issue.
• Added some comments to clarify some tests.
• Added // FIXME: Remove after GH-77101 is fixed. to some input tests, since a new action should not need to be started in between edits with different carets. related #77101.
• Removed caret index edit order tests, Added sort carets and merge carets tests.
• Added text manipulation test case with subcases backspace, new line,move lines up,move lines down,delete lines,duplicate selection,duplicate lines.

[AThousandShips]

You're breaking compatibility and need to implement compatibility methods, if you need help I can instruct when I have the time 🙂

Here you go:

diff --git a/misc/extension_api_validation/4.2-stable.expected b/misc/extension_api_validation/4.2-stable.expected
index a8b3af7891..49d7841693 100644
--- a/misc/extension_api_validation/4.2-stable.expected
+++ b/misc/extension_api_validation/4.2-stable.expected
@@ -28,3 +28,11 @@ GH-86687
 Validate extension JSON: Error: Field 'classes/AnimationMixer/methods/_post_process_key_value/arguments/3': type changed value in new API, from "Object" to "int".

 Exposing the pointer was dangerous and it must be changed to avoid crash. Compatibility methods registered.
+
+
+GH-86978
+--------
+Validate extension JSON: Error: Field 'classes/TextEdit/methods/set_line/arguments': size changed value in new API, from 2 to 3.
+Validate extension JSON: Error: Field 'classes/TextEdit/methods/swap_lines/arguments': size changed value in new API, from 2 to 3.
+
+YOUR DESCRIPTION. Compatibility methods registered.
diff --git a/scene/gui/text_edit.compat.inc b/scene/gui/text_edit.compat.inc
new file mode 100644
index 0000000000..708b9d9a9d
--- /dev/null
+++ b/scene/gui/text_edit.compat.inc
@@ -0,0 +1,46 @@
+/**************************************************************************/
+/*  text_edit.compat.inc                                                  */
+/**************************************************************************/
+/*                         This file is part of:                          */
+/*                             GODOT ENGINE                               */
+/*                        https://godotengine.org                         */
+/**************************************************************************/
+/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
+/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur.                  */
+/*                                                                        */
+/* Permission is hereby granted, free of charge, to any person obtaining  */
+/* a copy of this software and associated documentation files (the        */
+/* "Software"), to deal in the Software without restriction, including    */
+/* without limitation the rights to use, copy, modify, merge, publish,    */
+/* distribute, sublicense, and/or sell copies of the Software, and to     */
+/* permit persons to whom the Software is furnished to do so, subject to  */
+/* the following conditions:                                              */
+/*                                                                        */
+/* The above copyright notice and this permission notice shall be         */
+/* included in all copies or substantial portions of the Software.        */
+/*                                                                        */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,        */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF     */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY   */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,   */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE      */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                 */
+/**************************************************************************/
+
+#ifndef DISABLE_DEPRECATED
+
+void TextEdit::_set_line_bind_compat_86978(int p_line, const String &p_new_text) {
+       set_line(p_line, p_new_text, true);
+}
+
+void TextEdit::_swap_lines_bind_compat_86978(int p_from_line, int p_to_line) {
+       swap_lines(p_from_line, p_to_line, true);
+}
+
+void TextEdit::_bind_compatibility_methods() {
+       ClassDB::bind_compatibility_method(D_METHOD("set_line", "line", "new_text"), &TextEdit::_set_line_bind_compat_86978);
+       ClassDB::bind_compatibility_method(D_METHOD("swap_lines", "from_line", "to_line"), &TextEdit::_swap_lines_bind_compat_86978);
+}
+
+#endif // DISABLE_DEPRECATED
diff --git a/scene/gui/text_edit.cpp b/scene/gui/text_edit.cpp
index e774722c86..ab6333ef0e 100644
--- a/scene/gui/text_edit.cpp
+++ b/scene/gui/text_edit.cpp
@@ -29,6 +29,7 @@
 /**************************************************************************/

 #include "text_edit.h"
+#include "text_edit.compat.inc"

 #include "core/config/project_settings.h"
 #include "core/input/input.h"
diff --git a/scene/gui/text_edit.h b/scene/gui/text_edit.h
index 8fdf05c73e..09de71dfc9 100644
--- a/scene/gui/text_edit.h
+++ b/scene/gui/text_edit.h
@@ -624,6 +624,12 @@ protected:
        void _notification(int p_what);
        static void _bind_methods();

+#ifndef DISABLE_DEPRECATED
+       void _set_line_bind_compat_86978(int p_line, const String &p_new_text);
+       void _swap_lines_bind_compat_86978(int p_from_line, int p_to_line);
+       static void _bind_compatibility_methods();
+#endif // DISABLE_DEPRECATED
+
        virtual void _update_theme_item_cache() override;

        /* Internal API for CodeEdit, pending public API. */

[AThousandShips]

Many of the comment changes here are in unrelated areas of code and while appreciated they do complicate the git history, I think it should be limited to areas where you're already changing things to avoid chaos

[kitbdev]

Added compat file and removed comment changes.

[MewPurPur]

I'm insanely impressed and very excited about this.

I just want to point out that LineEdit has similar problems around its text selection logic, so we'll need to address that too, down the... line.

[MewPurPur]

is_selection_direction_right -> is_caret_after_selection_origin ?

[kitbdev]

Is there any chance you could spilt some of these change up into multiple PR / commits?

This is tricky since the multicaret edit change and the selection change require lots of functions to be rewritten, and some of them depend on each other. I think I can split out some smaller parts into separate PRs, like the queue_redraw change and the IME changes. This PR would probably depend on them.

If we're trying to simply the interface could we always call merge_overlapping_carets?

I was thinking that users may still want to force a merge during a multicaret edit, but I'm not sure if there is a use case for that so I think we can remove queue_merge_carets() and put the functionality in merge_overlapping_carets().

via collapse_carets (should this be exposed?)

It doesn't need to be exposed since all methods that remove text call it internally. It's only public because it's useful for CodeEdit::fold_line(). If there are more uses for it like that, then we can expose it.

The intention behind this was to make editing consistent regardless of order the carets where placed down. Plus processing carets twice. I guess due to delaying the merge / ignore carets this becomes a non-issue?

Yes, with the delayed merge, ignored carets, and removing text updating carets after it, the order no longer has to be from bottom to top and can be in any order.

Would we want to force all multicaret edits to be complex operations? Otherwise the undo / redo will not be great. Though you might want a complex op to consist of multiple multicaret edits, so don't think we can combine them entirely.

There are also some places where we want a multicaret edit to not be part of undo / redo at all, like add_caret_at_carets, so we can't combine them.

is_selection_direction_right -> is_caret_after_selection_origin ?

is_caret_after_selection_origin is more accurate, so I think I'll use this.

Edit: Apparently hitting ctrl+shift+enter closes with comment, oops.

[kitbdev]

Fixed merge conflict, combined queue_merge_carets() and merge_overlapping_carets(), and renamed is_selection_direction_right to is_caret_after_selection_origin.

I'm going to try and extract some smaller PRs that can be merged more easily.

[MewPurPur]

I'm testing this PR right now to help get it through. Should operations always have the same effect, regardless of the order in which you created carets?

[MewPurPur]

I'll assume yes to my above post and report my findings, I don't wanna delay them. I've done a lot of testing and found very few new issues. (Except for the first one, that one might not be new. Old backspace was generally buggy)

lmofa
|lmo|fa

The result after pressing ui_text_backspace_word (Ctrl+Backspace) or ui_text_backspace_all_to_left (no default shortcut) is different depending on the order of placing down the carets.

fun|c lmofa():|
  pa|ss

The result after pressing Fold/Unfold Line (Alt+F) is an error in output.

---

Ctrl+Shift+U for inputting unicode has several new bugs. On success, Ctrl+Shift+U no longer deletes the unicode stuff after you're done typing. With multicaret, carets easily get misplaced. If you do it with a selection, Godot pretty much breaks.

[kitbdev]

The backspace issue also happened with ui_text_delete_word when one of the carets was at the end of the line and the other on the last word. It doesn't apply to ui_text_delete_all_to_right though since it doesn't delete newlines. I fixed the issue by making sure newlines are handled after removing text for the line, and added tests for it. Fixed the fold line issue and added a test for it.

I couldn't find what you meant for the unicode issue. For me Ctrl+Shift+U isn't bound to anything and I didn't see any actions or functions about unicode that would function like that. The only unicode input I know is holding ALT and typing + and the code, like +1F600. But it seems to be working fine.

[MewPurPur]

Oh, so maybe Ctrl+Shift+U is linux-specific? I was discussing it with a friend who's on Windows once, I thought it's universal.

Anyway, when you do this shortcut, an underlined "u" appears, and then you write a 4-letter code and get a unicode character when you press Enter.

This worked before somewhat ok, now it works less okay.

[kitbdev]

I think it must be using the IME system. I fixed the selected text issue and extracted all IME changes to #87479. Reverted all IME changes here, it should work the same as it did in master.

[Paulb23]

I think I can split out some smaller parts into separate PRs, like the queue_redraw change and the IME changes. This PR would probably depend on them.

As many as you feel happy to would be appreciated, dependent PRs are fine as they'll hopefully be smaller so can be merged quicker (with a little reabasing), or even creating separate commits would be okay!

If there are more uses for it like that, then we can expose it.

I'm leaning towards adding any public method to the API by default so we don't have too much "editor" only code if we can help it. As if we're making it public then there's a need for it somewhere. If there's really no use for it being called outside TextEdit / CodeEdit though, protected is fine (particularly around folding as that is awkwardly split at the moment).

There are also some places where we want a multicaret edit to not be part of undo / redo at all, like add_caret_at_carets, so we can't combine them.

Calling begin/end_multicaret_edit() here seems a little odd to me, as adding / removing carets shouldn't be changing any text? Looking at add_caret_at_carets in particular, it calls merge_overlapping_carets at the end which should achieve the same purpose?

[kitbdev]

I split the commit into two parts:

• Selection overhaul - includes changes to selection, mouse handling, set_caret_line -1, and goto line changes.
• Multicaret edit overhaul - includes multicaret edits, everything else.

It's not perfect since I don't think the first commit could be built by itself and most things are just in the last commit, and I may have missed some things. Hopefully it helps with reviewing the code. I'll see if I can extract anything else. I can also remerge them later, if we want.

I'm leaning towards adding any public method to the API by default

In this case, I will make _is_line_col_in_range and _selection_contains private, since they aren't needed elsewhere, and I'll expose get_selection_at and collapse_carets. I wasn't able to expose get_line_ranges_from_carets, since I got compiler errors, I guess Vector<Point2i> can't be exposed?

Calling begin/end_multicaret_edit() here seems a little odd to me, as adding / removing carets shouldn't be changing any text? Looking at add_caret_at_carets in particular, it calls merge_overlapping_carets at the end which should achieve the same purpose?

Multicaret edits don't need to involve changing any text, only when carets are changed and iterating over them. I made add_caret always able to return a caret if in a multicaret edit, and it will be merged later. I needed to be able to add a caret even if it overlaps, so I can set the selection so it can get merged properly.

A better example would probably be CodeEdit::toggle_foldable_line_for_all_carets() which doesn't change text but needs to collapse carets without immediately merging them, and not be part of undo / redo.

[kitbdev]

Fixed failing check and added collapse tests.

[Paulb23]

Appreciate the split.

I wasn't able to expose get_line_ranges_from_carets, since I got compiler errors, I guess Vector<Point2i> can't be exposed?

Yeah can't expose Vector, have convert it to a TypedArray to do that.

A better example would probably be CodeEdit::toggle_foldable_line_for_all_carets() which doesn't change text but needs to collapse carets without immediately merging them, and not be part of undo / redo.

I see, that makes sense. I guess it might be worth adding a note to the docs that this doesn't replace complex op calls but rather an addition. Though in this case an example might be clearer?

[kitbdev]

Yeah can't expose Vector, have convert it to a TypedArray to do that.

If I use a TypedArray, I think I won't be able to use ranged for each loops with it. Also, it looks like there is no PackedVector2iArray for some reason.

I see, that makes sense. I guess it might be worth adding a note to the docs that this doesn't replace complex op calls but rather an addition. Though in this case an example might be clearer?

I guess I thought it would be clear they are unrelated, but an example would be useful anyway.

Something like:

begin_complex_operation()
begin_multicaret_edit()
for i in range(get_caret_count()):
	if multicaret_edit_ignore_caret(i):
		continue
	# Logic here.
end_multicaret_edit()
end_complex_operation()

[kitbdev]

Rebased to fix minor merge conflicts and update to new deprecated doc style. Also, I think I can split off a few more things into separate PRs so its easier to review.

[MewPurPur]

I think this PR is clear enough as is. I gave it a big 30min round of testing, trying out every functionality I could think of, I couldn't get any bugs to happen.

[kitbdev]

Rebased to fix conflicts (#88474). I removed the save multiple carets state and the goto line changes, since I wanted to make some changes to them and they can be done separately. I'll make a new PR for them, they will likely depend on this one.

[Mickeon]

Going to chime in and say that this would also fix https://github.com/godotengine/godot/issues/76093 . What an insane list of bug reports

[kitbdev]

Going to chime in and say that this would also fix https://github.com/godotengine/godot/issues/76093

This doesn't actually fix https://github.com/godotengine/godot/issues/76093. This PR should fix all other issues that were fixed in https://github.com/godotengine/godot/pull/73580, except for that one. I didn't fix https://github.com/godotengine/godot/issues/76093 because it seemed like a bigger issue since it also affects word left/right behavior and may affect TS->shaped_text_get_word_breaks, but I didn't really look into it.

Also rebased to fix minor merge conflict.

[kitbdev]

• Rebased and fixed minor conflicts after https://github.com/godotengine/godot/pull/85325.
• Added https://github.com/godotengine/godot/issues/89417 to the fix list.

[kitbdev]

Rebased to fix conflicts. Removed args for set_line and swap_lines so they don't break compat, and other suggestions.

• Edit: added https://github.com/godotengine/godot/issues/90462 to the fix list

[kitbdev]

rebased to fix merge conflicts

[akien-mga]

Great work! Thanks for putting up with the exhaustive review process :)