Add Duplicate Lines shortcut to CodeTextEditor #66553

PucklaJ · 2022-09-28T15:27:35Z

This PR adds the shortcut script_text_editor/duplicate_lines. The default buttons are Ctrl+Alt+Down. For macOS the default buttons are Cmd+Shift+Down. These shortcuts are similiar to the VS Code command editor.action.copyLinesDownAction. If no text is selected it behaves the same as duplicate_selection, but if text is selected it will duplicate the whole lines of the selection.
This also supports multiple carets.

script_text_editor/duplicate_lines:

script_text_editor/duplicate_selection:

PucklaJ · 2022-10-09T10:18:02Z

Added support for multiple carets

MewPurPur · 2023-04-15T19:34:46Z

I'll do a code review in a sec, but can you argue for why you think this feature should be added? We don't accept features just because PRs exist for them.

I personally think Ctrl+Shift+D does what I want just fine. If I have no selections, this PR does the same, just adjusts the caret in one of the scenarios. If I want to duplicate multiple lines, I'm just going to do a proper selection with triple-clicking and then dragging to a line's end - both easy and very well-known functionalities.

MewPurPur

Haven't tested the feature itself, just a code review.

editor/code_editor.cpp

MewPurPur · 2023-04-15T19:56:43Z

editor/code_editor.cpp

+		int select_from_column = -1;
+		int select_to_column = -1;


select_from_column and select_to_column are unnecessary, this is line duplication. You only used this variable once.

I tried to remove them, but when the text gets inserted, the selection gets deleted, which means that I need to store them before changing the caret. And I think that I should transfer the selection to the duplication since it means that you can perform the same duplication multiple times in a row. Which is something I use very often.

editor/code_editor.cpp

MewPurPur · 2023-04-15T20:15:35Z

editor/code_editor.cpp

+		int select_from_line = -1;
+		int select_to_line = -1;


You don't use these much outside of places where has_selection() is checked. You should define them close to the scope where the variables have a meaning.

If I put them in their own scope I would need to write duplicate code.

torcado194 · 2023-04-15T20:27:05Z

I would love this feature. I have never once wanted to duplicate a subsection within a line using a hotkey, especially if that selection spans multiple lines, I couldn't imagine a scenario in which I would want to duplicate that content in-place.
I duplicate lines constantly, duplicating and swapping lines are probably my most used hotkeys aside from undo/redo. Needing to make sure my selections encompass the entirety of the lines I want to duplicate before i'm able to duplicate multiple lines is very tedious, especially with no way to automatically spread selections to the start/end of lines. If a cursor in my multi-cursor selection started in the middle of the line? Now i have to redo the whole selection to move the start point to the beginning of the line.
And a mouse-based alternative isn't ideal. I'm already typing, I want to use the keyboard, not the mouse. At least for something this simple.

MewPurPur · 2023-04-16T01:15:32Z

@torcado194 That makes sense, I think "Duplicate Lines" is a hard thing to argue against actually. What about this particular implementation, with separate "below" and "above" versions? That's the main part I find unnecessary.

torcado194 · 2023-04-16T19:24:48Z

I agree, I hardly ever use "duplicate above". "below" is the default direction for the duplicate line action for any editor I use (and is what I'd expect). the "duplicate above" result can be easily substituted using the keyboard by duplicating below and swapping the new selected lines upward, at the cost of additional keystrokes equal to the line count.
but on the other hand the duplicate above and below functions are found in many editors I've used (visual studio and vs code for example)
maybe it's not part of godot's philosophy but I would much rather have more hotkey functions than less, even if they're unassigned by default.

PucklaJ · 2023-04-17T10:40:38Z

What about this particular implementation, with separate "below" and "above" versions?

I tried to recreate the feature from VS Code and there they have both a above and below version. But I agree I hardly use it either.

PucklaJ · 2023-04-18T09:43:50Z

I will merge all commits into one, once you are done reviewing

PucklaJ · 2023-08-18T07:41:51Z

@MewPurPur @Paulb23 This PR has just been hanging here for quite some time. Can you please finish reviewing it. I would really like it to be merged. I think it adds some good keyboard shortcuts which can really improve your productivity.

MewPurPur · 2023-08-18T10:12:03Z

I still think it should be just "Duplicate Lines" that does it below, instead of above and below versions. It's not like regular duplicate is "before" and "after" I don't got much else to say, the code to me looks good and my remarks have been clarified or addressed.

PucklaJ · 2023-08-26T16:28:07Z

That said, before this can be merged you need to squash your commits.

Done

I also dropped the commit which replaces the duplicate_selection shortcut so that it doesn't break compatibility.

Paulb23

Code looks fine, few minor comments.

I'm wanting to move things like this into CodeEdit, could then take advantage of unit tests as well. If you fancy doing that :)

Paulb23 · 2023-08-28T14:15:52Z

editor/code_editor.cpp

+		text_editor->deselect(caret_index);
+
+		text_editor->insert_text_at_caret(insert_text, caret_index);
+		text_editor->set_caret_line(new_caret_line, true, true, 0, caret_index);


Should set adjust_veiwport to false here, and leave it up to set_caret_column

Paulb23 · 2023-08-28T14:17:15Z

editor/code_editor.cpp

+		}
+	}
+
+	text_editor->merge_overlapping_carets();


Are we expecting carets to overlap here?

Paulb23 · 2023-08-28T14:17:54Z

editor/code_editor.cpp

+		}
+
+		// The text will be inserted at the end of the current line.
+		text_editor->set_caret_column(text_editor->get_line(text_editor->get_caret_line(caret_index)).length(), true, caret_index);


No need to adjust_veiwport here either

Paulb23 · 2023-08-28T14:19:36Z

editor/code_editor.cpp

+			new_caret_line = select_to_line + select_num_lines;
+
+			for (int i = select_from_line; i <= select_to_line; i++) {
+				text_editor->unfold_line(i);


Can unfold in the same loop above

PucklaJ · 2023-08-29T09:50:41Z

@Paulb23 I moved the code into CodeEdit and then adjusted the code based on your review.

Paulb23

Thanks!

One small change, could you bind it to to the CodeEdit API? and again would appreciate any unit tests.

scene/gui/code_edit.h

PucklaJ · 2023-09-16T05:45:53Z

@Paulb23 After a rebase and fixing a merge conflict I added unit tests and bound duplicate_lines to the CodeEdit API.

scene/gui/code_edit.cpp

doc/classes/CodeEdit.xml

YuriSizov · 2023-09-25T18:57:04Z

editor/plugins/script_text_editor.cpp

@@ -2395,6 +2399,8 @@ void ScriptTextEditor::register_editor() {
 	ED_SHORTCUT("script_text_editor/unfold_all_lines", TTR("Unfold All Lines"), Key::NONE);
 	ED_SHORTCUT("script_text_editor/duplicate_selection", TTR("Duplicate Selection"), KeyModifierMask::SHIFT | KeyModifierMask::CTRL | Key::D);
 	ED_SHORTCUT_OVERRIDE("script_text_editor/duplicate_selection", "macos", KeyModifierMask::SHIFT | KeyModifierMask::META | Key::C);
+	ED_SHORTCUT("script_text_editor/duplicate_lines", TTR("Duplicate Lines"), KeyModifierMask::CMD_OR_CTRL | KeyModifierMask::ALT | Key::DOWN);
+	ED_SHORTCUT_OVERRIDE("script_text_editor/duplicate_lines", "macos", KeyModifierMask::CMD_OR_CTRL | KeyModifierMask::ALT | Key::Z);


Is this a common macOS shortcut for this purpose?

I don't know. Honestly I just put anything there because it would have been a duplicate otherwise

Would be good to know what's the expected macOS shortcut for this operation (e.g. in VS Code or Xcode). CC @bruvzg @coppolaemilio

I just looked it up. On VS Code its shift+cmd+down

That might work. Seems like we use this mapping but only for PC here, and there's a different override for macOS:

// core/input/input_map.cpp inputs = List<Ref<InputEvent>>(); inputs.push_back(InputEventKey::create_reference(Key::DOWN | KeyModifierMask::SHIFT | KeyModifierMask::CMD_OR_CTRL)); default_builtin_cache.insert("ui_text_caret_add_below", inputs); inputs = List<Ref<InputEvent>>(); inputs.push_back(InputEventKey::create_reference(Key::L | KeyModifierMask::SHIFT | KeyModifierMask::CMD_OR_CTRL)); default_builtin_cache.insert("ui_text_caret_add_below.macos", inputs);

I changed it on macos to cmd_or_ctrl+shift+down

YuriSizov

Consensus seems to be for this, at a glance the style looks fine.

YuriSizov · 2023-09-25T18:59:37Z

Could you please squash your commits into one? Make sure that the final commit has a short but descriptive message (the title of this PR is a good option). See this documentation, if you need help with squashing.

PucklaJ · 2023-09-25T20:57:55Z

@YuriSizov @Paulb23 @MewPurPur The commits are squashed. Everything is ready. Thank you for approving this PR.

This keyboard shortcut has been made with inspiration from the VS Code keyboard shortcut editor.action.copyLinesDownAction. It duplicates all selected lines and inserts them below no matter where the caret is within the line.

akien-mga · 2023-09-26T06:37:03Z

Thanks!

PucklaJ requested review from a team as code owners September 28, 2022 15:27

PucklaJ force-pushed the duplicate_lines branch from 5a01ec3 to bc668c4 Compare September 28, 2022 18:15

Calinou added enhancement topic:editor usability topic:gui labels Sep 29, 2022

Calinou added this to the 4.0 milestone Sep 29, 2022

PucklaJ force-pushed the duplicate_lines branch 2 times, most recently from 65c9a01 to 711307d Compare October 9, 2022 09:59

PucklaJ force-pushed the duplicate_lines branch 2 times, most recently from 919f7d7 to e95dc4f Compare October 18, 2022 09:44

YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023

MewPurPur suggested changes Apr 15, 2023

View reviewed changes

PucklaJ force-pushed the duplicate_lines branch from e95dc4f to 4c7c4ca Compare April 18, 2023 09:43

YuriSizov requested a review from Paulb23 April 18, 2023 16:11

PucklaJ force-pushed the duplicate_lines branch from 4c7c4ca to 98cb988 Compare April 21, 2023 08:02

akien-mga modified the milestones: 4.1, 4.x Jun 19, 2023

PucklaJ force-pushed the duplicate_lines branch from 98cb988 to 731968a Compare August 18, 2023 14:39

PucklaJ force-pushed the duplicate_lines branch from d9cbdad to fef53e8 Compare August 26, 2023 16:27

Paulb23 reviewed Aug 28, 2023

View reviewed changes

PucklaJ requested a review from a team as a code owner August 29, 2023 09:49

Paulb23 reviewed Sep 3, 2023

View reviewed changes

scene/gui/code_edit.h Show resolved Hide resolved

AThousandShips changed the title ~~Add Duplicate Lines Below and Above shortcut to CodeTextEditor~~ Add Duplicate Lines shortcut to CodeTextEditor Sep 3, 2023

PucklaJ force-pushed the duplicate_lines branch from 5baed4e to 2c4dd12 Compare September 16, 2023 05:44

PucklaJ requested a review from a team as a code owner September 16, 2023 05:44

PucklaJ requested a review from a team as a code owner September 16, 2023 06:04

Paulb23 reviewed Sep 17, 2023

View reviewed changes

scene/gui/code_edit.cpp Show resolved Hide resolved

PucklaJ force-pushed the duplicate_lines branch from b8c78df to ba799fc Compare September 21, 2023 17:18

AThousandShips reviewed Sep 21, 2023

View reviewed changes

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

YuriSizov reviewed Sep 25, 2023

View reviewed changes

YuriSizov approved these changes Sep 25, 2023

View reviewed changes

YuriSizov modified the milestones: 4.x, 4.2 Sep 25, 2023

PucklaJ force-pushed the duplicate_lines branch from 00d7bf4 to 64bcb2f Compare September 25, 2023 20:56

Add Duplicate Lines shortcut to CodeTextEditor

d2e651f

This keyboard shortcut has been made with inspiration from the VS Code keyboard shortcut editor.action.copyLinesDownAction. It duplicates all selected lines and inserts them below no matter where the caret is within the line.

PucklaJ force-pushed the duplicate_lines branch from 64bcb2f to d2e651f Compare September 25, 2023 21:41

akien-mga merged commit 1aa2d8b into godotengine:master Sep 26, 2023
15 checks passed

PucklaJ deleted the duplicate_lines branch September 26, 2023 10:27

akien-mga added topic:gui and removed topic:gui labels Oct 3, 2023

kitbdev mentioned this pull request Oct 18, 2023

Script Text Editor Duplicate Lines shortcut acts wrong when cursor is at the top #83589

Closed

kitbdev mentioned this pull request Jan 8, 2024

Overhaul multicaret editing and selection in TextEdit #86978

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Duplicate Lines shortcut to CodeTextEditor #66553

Add Duplicate Lines shortcut to CodeTextEditor #66553

PucklaJ commented Sep 28, 2022 •

edited

Loading

PucklaJ commented Oct 9, 2022 •

edited

Loading

MewPurPur commented Apr 15, 2023 •

edited

Loading

MewPurPur left a comment •

edited

Loading

MewPurPur Apr 15, 2023

PucklaJ Apr 18, 2023 •

edited

Loading

MewPurPur Apr 15, 2023

PucklaJ Apr 18, 2023

torcado194 commented Apr 15, 2023

MewPurPur commented Apr 16, 2023 •

edited

Loading

torcado194 commented Apr 16, 2023

PucklaJ commented Apr 17, 2023

PucklaJ commented Apr 18, 2023

PucklaJ commented Aug 18, 2023

MewPurPur commented Aug 18, 2023

PucklaJ commented Aug 26, 2023 •

edited

Loading

Paulb23 left a comment

Paulb23 Aug 28, 2023

Paulb23 Aug 28, 2023

Paulb23 Aug 28, 2023

Paulb23 Aug 28, 2023

PucklaJ commented Aug 29, 2023

Paulb23 left a comment

PucklaJ commented Sep 16, 2023 •

edited

Loading

YuriSizov Sep 25, 2023

PucklaJ Sep 25, 2023

akien-mga Sep 25, 2023

PucklaJ Sep 25, 2023

akien-mga Sep 25, 2023

PucklaJ Sep 25, 2023

YuriSizov left a comment

YuriSizov commented Sep 25, 2023

PucklaJ commented Sep 25, 2023

akien-mga commented Sep 26, 2023

Add Duplicate Lines shortcut to CodeTextEditor #66553

Add Duplicate Lines shortcut to CodeTextEditor #66553

Conversation

PucklaJ commented Sep 28, 2022 • edited Loading

PucklaJ commented Oct 9, 2022 • edited Loading

MewPurPur commented Apr 15, 2023 • edited Loading

MewPurPur left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PucklaJ Apr 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torcado194 commented Apr 15, 2023

MewPurPur commented Apr 16, 2023 • edited Loading

torcado194 commented Apr 16, 2023

PucklaJ commented Apr 17, 2023

PucklaJ commented Apr 18, 2023

PucklaJ commented Aug 18, 2023

MewPurPur commented Aug 18, 2023

PucklaJ commented Aug 26, 2023 • edited Loading

Paulb23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PucklaJ commented Aug 29, 2023

Paulb23 left a comment

Choose a reason for hiding this comment

PucklaJ commented Sep 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YuriSizov left a comment

Choose a reason for hiding this comment

YuriSizov commented Sep 25, 2023

PucklaJ commented Sep 25, 2023

akien-mga commented Sep 26, 2023

PucklaJ commented Sep 28, 2022 •

edited

Loading

PucklaJ commented Oct 9, 2022 •

edited

Loading

MewPurPur commented Apr 15, 2023 •

edited

Loading

MewPurPur left a comment •

edited

Loading

PucklaJ Apr 18, 2023 •

edited

Loading

MewPurPur commented Apr 16, 2023 •

edited

Loading

PucklaJ commented Aug 26, 2023 •

edited

Loading

PucklaJ commented Sep 16, 2023 •

edited

Loading