Add Multi-caret support to TextEdit #61902

Paulb23 · 2022-06-10T13:46:46Z

This PR implements the initial support for multiple carets in TextEdit. Adjusted CodeEdit and the Editor to support them.

This will most likely be the last major feature that I have time to make for 4.0.

Will look at adding multi-caret tests in a separate PR(s), as those will take a little while as this change is quite large.

For the initial support currently it's pretty bare bones, it's enabled by default. You can add an additional caret by Alt + LeftClick. Typing will insert at all carets etc. Caret '0' is the main caret, the viewport will always follow this caret unless selecting via the mouse with a secondary caret. Each caret also has it's own selection.

I've tried to make the behaviour when using multiple carets intuitive based on VSCode, SublimeText and QTCreator, but even between them there are a lot of differences and bugs. Not sure on performance with a lot of carets due to the checks etc but happy to optimise after.

In terms of API changes most of the viewport, caret and edit methods now take in a caret index, the caret index is one of the following:

-1 for all carets
0 for the main caret
1-x for the rest

Each method is defaulted as appropriate so existing code should still work (as seen in current tests).

Each method will only accept -1 where it makes sense. For example has_selection(-1) will return true if any caret has a selection. Whereas get_caret_line does not have a strong use case for supporting -1, so is defaulted to 0.

Some utility methods have been exposed for handing multiple carets, they are as follows:
- add_caret - Adds a caret.
- remove_caret - Removes a caret.
- get_caret_count - Gets the number of carets.
- remove_secondary_carets - Removes all additional carets.
- merge_overlapping_carets - Merges any carets and selections that are overlapping. Note this is not called when a caret changes position but after "actions", so if using the API it is possible to get into a state where carets do overlap.
- get_caret_index_edit_order - Returns a list of caret indexes in their edit order, this done from bottom to top. Edit order refers to the way action such as insert_text_at_caret are applied.
- adjust_carets_after_edit - Should be called after editing a caret to reposition the carets affected by the edit such as adding a new line will need to push all carets and selection after that point down a line. This assumes edits are applied in edit order.

Do due a typing now having the potential to consist of multiple edits, the undo redo system would put each caret as a separate step. Using [begin|end]_complex_operation around this would force it into a single step. However this would only make each letter when typing a step, the typing session would not be. We needed a way to have a "hanging" complex op, this comes in the form of an "action".

Calling start_action will end the current action if it is not the same, before starting the new one. end_action marks that the any changes from the action are over, however the action does not end until the timeout or another action is started. This can be done via directly calling start_action, [begin|end]_complex_operation or via a edit that triggers and undo redo step outside of the [start|end]_action calls.

There are currently four supported actions, intended as follow:
- None - No current action.
- Typing - Normal typing
- Backspace - A backwards delete typically using the backspace
- Delete - A forwards delete typically using the delete key.

This fixes some old issues where typing when the caret had a selection would be treated as an entire step, so completion would not work etc. Similarly, pressing the delete key would create an undo step for each letter, now they are treated like typing where multiple deletes are a single action.

Final notes on the undo redo, the system used to use a best guess approach to determine where the caret went, it will now restore the carets exactly as they were. One thing to watch out for, is many carets can spam the undo stack as it becomes difficult to merge them into a single item, as they move about due to other edits, this may cause the undo stack to fill up pretty fast.

Lastly, in order to get this is work, moved the viewport variables outside of the caret and gave them some slightly better names, and currently only the main caret is restored on launch.

closes godotengine/godot-proposals/issues/245
closes godotengine/godot-proposals/issues/3455

mhilbrunner · 2022-08-18T19:46:41Z

Editor PR review meeting: Needs review/testing, really nice and wanted feature though

MewPurPur · 2022-10-03T02:01:56Z

Thanks for making this! I haven't really used mutli-carets on any IDE and I don't have any stylistic prejudices. Also since it's an initial work, I'll not discuss usability.

So here's just a good ol' dump of bugs instead:

If the main caret enters some other caret's selection and gets merged with it, the viewport doesn't follow it until there's another movement.
When making a mouse selection (Shift + Click), if there are multiple carets that would end up inside the selection, only the first such caret gets destroyed. The other carets can even move a bit before realizing their existence is a mistake, resulting in double-selection :P
If two carets are a character apart, Backspace usually makes them align.
In the above scenario, if one of the carets is at the beginning of a line, `Backspace sends the other caret at the same column in the previous line.
Alt + Up (Move Up) is broken for multiple carets.
Alt + F (Fold Line) only works for the main caret.
Ctrl+Shift+D (Duplicate Line) makes the first caret duplicate the second caret's line.
No one's gonna ever use multi-carets for this, but for completion... Ctrl+Shift+E (Evaluate Expression) puts every evaluated value at each caret.

Paulb23 · 2022-10-03T19:09:35Z

Rebased and thanks for testing, hopefully fixed most of those issues.

Regarding Fold Line and Evaluate Expression for initial support I think that's okay, can enhance them later.

KoBeWi · 2022-10-04T20:56:26Z

editor/code_editor.cpp

+	Vector<int> cursor_columns;
+	for (int c = 0; c < text_editor->get_caret_count(); c++) {
+		cursor_columns.push_back(text_editor->get_caret_column(c));
+	}


KoBeWi · 2022-10-04T21:10:35Z

Should paste behave this way?

VS Code for comparison

KoBeWi · 2022-10-04T21:13:30Z

scene/gui/code_edit.cpp

-			remove_text(get_caret_line(), get_caret_column(), get_caret_line(), get_caret_column() + 1);
+		// Remove the old character if in overtype mode and no selection.
+		if (is_overtype_mode_enabled() && !had_selection) {
+			/* Make sure we don't try and remove empty space. */


Suggested change

/* Make sure we don't try and remove empty space. */

// Make sure we don't try and remove empty space.

KoBeWi · 2022-10-04T21:29:12Z

scene/gui/code_edit.cpp

+		/* This value informs us by how much we changed selection position by indenting right. */
+		/* Default is 1 for tab indentation.                                                   */


Suggested change

/* This value informs us by how much we changed selection position by indenting right. */

/* Default is 1 for tab indentation. */

// This value informs us by how much we changed selection position by indenting right.

// Default is 1 for tab indentation.

This comment style is just used everywhere in the codebase.

Same below.

KoBeWi · 2022-10-04T21:53:59Z

scene/gui/text_edit.cpp

-			set_caret_line(caret.line - 1);
-			set_caret_column(text[caret.line].length());
+	for (int i = 0; i < carets.size(); i++) {
+		// Handle selection


Suggested change

// Handle selection

// Handle selection.

Could as well fix the comments if they are being moved.

KoBeWi · 2022-10-04T22:20:05Z

Ah right

In terms of API changes most of the viewport, caret and edit methods now take in a caret index, the caret index is one of the following:

-1 for all carets
0 for the main caret
1-x for the rest

This should be mentioned somewhere in the docs, probably in the class description or caret_multiple description.

KoBeWi · 2022-10-04T22:22:45Z

scene/gui/text_edit.cpp

-	String clipboard = text[cl];
-	DisplayServer::get_singleton()->clipboard_set(clipboard);
-	set_caret_column(0);
+		// Check for overlaping carrets


Suggested change

// Check for overlaping carrets

// Check for overlaping carrots

jk

KoBeWi · 2022-10-04T22:27:41Z

scene/gui/text_edit.h

@@ -42,6 +42,14 @@ class TextEdit : public Control {
 	GDCLASS(TextEdit, Control);

 public:
+	/* Edit Actions. */


Unnecessary comment.

...although I see there are similar comments below :/
It only repeats enum name.

KoBeWi · 2022-10-04T22:29:21Z

scene/gui/text_edit.h

@@ -361,19 +377,39 @@ class TextEdit : public Control {
 	int _get_char_pos_for_line(int p_px, int p_line, int p_wrap_index = 0) const;

 	/* Caret. */


I think this comment is out of place now, no?

KoBeWi · 2022-10-04T22:41:35Z

I checked through the code and it looks ok, I think. ~80% is just moving things around and adding caret index. Can't say much about the feature implementation, because it's rather complex, but I assume it's good, because it works xdd

I left some comments, mostly some useless bikeshedding about comments. We use // for comments, but I see lots of /* */ here, which isn't even used consistently. I think with that much code moved, we could take the chance to fix some of them.

From more important stuff, there is this weird paste behavior I mentioned above. Although I used VS Code as reference, so not sure how it works in other editors. #61902 (comment) is rather important too, because the PR just adds caret_index arguments everywhere without explaining what they do.

I don't use multiple carets much, but I think a nice-to-have feature would be rectangular selection:

It can be implemented in another PR.

EricEzaM · 2022-10-05T02:43:59Z

Rider has the same behaviour as VS code, I think it is the expected behaviour.

rider64_KWbv4gh2aZ.mp4

Paulb23 · 2022-10-05T16:28:28Z

Pasting should now work like VSCode, and updated the class description to have caret_index argument details.

doc/classes/TextEdit.xml

akien-mga · 2022-10-06T06:59:09Z

Thanks!

bruvzg · 2022-10-06T07:13:00Z

It's an edge case, but might need some extra indication (I do not know what exactly will be appropriate here):

If you have LTR segment in the RTL text (or RTL segment in LTR text), and two carets are on the both edges of it, both split carets are drawn on top of each other. So it's indistinguishable from singe caret on the segment edge.

Screen.Recording.2022-10-06.at.10.04.07.mov

Paulb23 added feature proposal topic:editor usability topic:gui labels Jun 10, 2022

Paulb23 added this to the 4.0 milestone Jun 10, 2022

Paulb23 requested review from a team as code owners June 10, 2022 13:46

Paulb23 force-pushed the multi-caret branch from 43012c1 to 493b3b9 Compare June 10, 2022 14:37

Paulb23 force-pushed the multi-caret branch from 493b3b9 to 496e235 Compare June 26, 2022 14:27

Paulb23 force-pushed the multi-caret branch 2 times, most recently from 3e84740 to c211042 Compare July 15, 2022 16:52

YuriSizov added the for pr meeting label Jul 21, 2022

Paulb23 force-pushed the multi-caret branch 2 times, most recently from a7e00e8 to be8ed21 Compare August 14, 2022 11:32

mhilbrunner added the needs testing label Aug 18, 2022

Paulb23 mentioned this pull request Aug 27, 2022

Implement a code formatter for GDScript in the script editor #55835

Closed

Paulb23 force-pushed the multi-caret branch from be8ed21 to 89ce034 Compare August 30, 2022 19:46

Mickeon mentioned this pull request Sep 21, 2022

[Reference PR] Attempt on adding mutli cursor to TextEdit #40195

Closed

Paulb23 force-pushed the multi-caret branch 2 times, most recently from d3f709e to 6f72a94 Compare September 30, 2022 16:33

Move selecion into caret

7dd3857

Paulb23 force-pushed the multi-caret branch from 6f72a94 to 2d115e2 Compare October 3, 2022 17:56

akien-mga requested review from bruvzg and KoBeWi October 4, 2022 08:51

KoBeWi reviewed Oct 4, 2022

View reviewed changes

Paulb23 added 3 commits October 5, 2022 17:19

Add mutliple Caret support to TextEdit

4a9d4e3

Add multi caret support to CodeEdit

e918024

Add multi caret support to Editor

0cbe176

Paulb23 force-pushed the multi-caret branch from bf53ec9 to 1494bc1 Compare October 5, 2022 16:20

KoBeWi reviewed Oct 5, 2022

View reviewed changes

doc/classes/TextEdit.xml Outdated Show resolved Hide resolved

KoBeWi approved these changes Oct 5, 2022

View reviewed changes

Update TextEdit docs

5f8f484

Paulb23 force-pushed the multi-caret branch from 1494bc1 to 5f8f484 Compare October 5, 2022 20:44

akien-mga merged commit f590321 into godotengine:master Oct 6, 2022

bruvzg mentioned this pull request Oct 7, 2022

Add split caret direction markers. Fix block/overtype caret size. #67016

Merged

dalexeev mentioned this pull request Oct 7, 2022

Add Editor Setting for default name when connecting signal to self #67018

Merged

PucklaJ mentioned this pull request Oct 9, 2022

Add Caret Insert Below and Above shortcuts to TextEdit #67139

Merged

3 tasks

Paulb23 deleted the multi-caret branch October 9, 2022 18:21

alfredbaudisch mentioned this pull request Oct 19, 2022

Add Selection and Caret for Next Occurrence of Selection (Select multiple occurrences of the same text and change them all at once) godotengine/godot-proposals#5621

Closed

Calinou removed the for pr meeting label Jan 20, 2023

kitbdev mentioned this pull request Jan 6, 2024

TextEdit multicaret and selection issues #86863

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Multi-caret support to TextEdit #61902

Add Multi-caret support to TextEdit #61902

Paulb23 commented Jun 10, 2022 •

edited

Loading

mhilbrunner commented Aug 18, 2022

MewPurPur commented Oct 3, 2022 •

edited

Loading

Paulb23 commented Oct 3, 2022

KoBeWi Oct 4, 2022

KoBeWi commented Oct 4, 2022

KoBeWi Oct 4, 2022

KoBeWi Oct 4, 2022 •

edited

Loading

KoBeWi Oct 4, 2022

KoBeWi commented Oct 4, 2022

KoBeWi Oct 4, 2022

KoBeWi Oct 4, 2022

KoBeWi Oct 4, 2022

KoBeWi commented Oct 4, 2022

EricEzaM commented Oct 5, 2022 •

edited

Loading

Paulb23 commented Oct 5, 2022

akien-mga commented Oct 6, 2022

bruvzg commented Oct 6, 2022

	/* Make sure we don't try and remove empty space. */
	// Make sure we don't try and remove empty space.

		/* This value informs us by how much we changed selection position by indenting right. */
		/* Default is 1 for tab indentation. */

	// Check for overlaping carrets
	// Check for overlaping carrots

		@@ -361,19 +377,39 @@ class TextEdit : public Control {
		int _get_char_pos_for_line(int p_px, int p_line, int p_wrap_index = 0) const;

		/* Caret. */

Add Multi-caret support to TextEdit #61902

Add Multi-caret support to TextEdit #61902

Conversation

Paulb23 commented Jun 10, 2022 • edited Loading

mhilbrunner commented Aug 18, 2022

MewPurPur commented Oct 3, 2022 • edited Loading

Paulb23 commented Oct 3, 2022

KoBeWi Oct 4, 2022

Choose a reason for hiding this comment

KoBeWi commented Oct 4, 2022

KoBeWi Oct 4, 2022

Choose a reason for hiding this comment

KoBeWi Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

KoBeWi Oct 4, 2022

Choose a reason for hiding this comment

KoBeWi commented Oct 4, 2022

KoBeWi Oct 4, 2022

Choose a reason for hiding this comment

KoBeWi Oct 4, 2022

Choose a reason for hiding this comment

KoBeWi Oct 4, 2022

Choose a reason for hiding this comment

KoBeWi commented Oct 4, 2022

EricEzaM commented Oct 5, 2022 • edited Loading

Paulb23 commented Oct 5, 2022

akien-mga commented Oct 6, 2022

bruvzg commented Oct 6, 2022

Paulb23 commented Jun 10, 2022 •

edited

Loading

MewPurPur commented Oct 3, 2022 •

edited

Loading

KoBeWi Oct 4, 2022 •

edited

Loading

EricEzaM commented Oct 5, 2022 •

edited

Loading