Script editor: Convert indentation on paste #28561
Conversation
|
This part of #19285 is not implemented BTW:
|
akien-mga
force-pushed
the
convert-indent-on-paste
branch
from
18328d8
to
7413052
Compare
akien-mga
changed the title
There was a problem hiding this comment.
Is it normal for other editors to detect space indent size, rather then using its own configuration? Both QT Creator and Sublime text don't seem to?
If not, I would suggest converting the existing methods in CodeTextEditor into text_edit with start / end line arguments (defaulting to the entire script), calling it after pasting.
Regardless, the setting will have to be created as a property of text_edit (defaulting to false), so it can be driven by CodeTextEditor, otherwise it will affect all text_edits in the editor, such as those in the inspector.
I've also seen this kind of paste separated into two shortcuts, a paste and a "paste with/without formatting", as an alternative to creating a setting.
In addition, there is an indent_using_spaces bool to detect whether it should be using spaces.
If it's true a string string called space_indent is padded with the number of spaces, so no need to recalculate these.
I tested with Kate, and it seems to detect indent size. The feature would quite limited otherwise IMO, as if you're pasting code indented with 6-spaces in a project with 4-space wide tab indentation, the expected behavior would be that 6 spaces become tabs, not only 4 of them with 2 spaces remaining. Similarly, if the code being pasted is indented with 8-spaces, I don't want 2 tabs per level. I tested this in Kate (6-spaces indentation): void some_method() {
if (true) {
print_line("Hello world");
}
}When pasting to a file configured to use tabs, it properly converts the 6 spaces to a tab, regardless of the configured tab width (4 or 8 chars). Kate is actually more clever than what I implemented here, as it manages to perfectly handle this mess: void some_method() {
if (true) {
print_line("Hello world");
}
return;
}
void some_method() {
if (true)
print_line("Hello world");
return;
}It properly indents everything with tabs based on the language semantics. Edit: So actually Kate likely doesn't try to figure out the indentation size by parsing the code, it just uses the language semantics to reflow everything as it ought to be. So since we don't do that, maybe we shouldn't try to figure things out and just stick to the configured indent width as QtCreator does. I tried QtCreator, and indeed it seems more limited, it only handles its configured indent width, and only when saving the file, not on paste.
Indeed, we could default to converting on paste, and add a "Paste with original formatting" context menu option. I'll have a look at making the architecture changes requested. |
Given the above, I think you're correct that we should likely not try to figure this out ourselves. And then,
Does sound like a better approach, reusing the methods called for |
|
@Paulb23 I did a quick test reusing The context menu "Paste" works fine, but the problem is that the Ctrl+V shortcut triggers diff --git a/editor/plugins/script_text_editor.cpp b/editor/plugins/script_text_editor.cpp
index 07303da2f..6869c76ab 100644
--- a/editor/plugins/script_text_editor.cpp
+++ b/editor/plugins/script_text_editor.cpp
@@ -1028,6 +1028,17 @@ void ScriptTextEditor::_edit_option(int p_op) {
} break;
case EDIT_PASTE: {
+ tx->paste();
+ // Convert clipboard indentation to match configured indent style
+ if (use_space_indentation) {
+ convert_indent_to_spaces();
+ } else {
+ convert_indent_to_tabs();
+ }
+ tx->call_deferred("grab_focus");
+ } break;
+ case EDIT_PASTE_ORIGINAL_INDENT: {
+
tx->paste();
tx->call_deferred("grab_focus");
} break;
@@ -1627,6 +1638,7 @@ void ScriptTextEditor::_make_context_menu(bool p_selection, bool p_color, bool p
}
context_menu->add_shortcut(ED_GET_SHORTCUT("script_text_editor/paste"), EDIT_PASTE);
+ context_menu->add_shortcut(ED_GET_SHORTCUT("script_text_editor/paste_original_indent"), EDIT_PASTE_ORIGINAL_INDENT);
context_menu->add_separator();
context_menu->add_shortcut(ED_GET_SHORTCUT("script_text_editor/select_all"), EDIT_SELECT_ALL);
context_menu->add_shortcut(ED_GET_SHORTCUT("script_text_editor/undo"), EDIT_UNDO);
@@ -1661,6 +1673,7 @@ void ScriptTextEditor::_make_context_menu(bool p_selection, bool p_color, bool p
ScriptTextEditor::ScriptTextEditor() {
theme_loaded = false;
+ use_space_indentation = EditorSettings::get_singleton()->get("text_editor/indent/type");
VSplitContainer *editor_box = memnew(VSplitContainer);
add_child(editor_box);
@@ -1725,6 +1738,7 @@ ScriptTextEditor::ScriptTextEditor() {
edit_menu->get_popup()->add_shortcut(ED_GET_SHORTCUT("script_text_editor/cut"), EDIT_CUT);
edit_menu->get_popup()->add_shortcut(ED_GET_SHORTCUT("script_text_editor/copy"), EDIT_COPY);
edit_menu->get_popup()->add_shortcut(ED_GET_SHORTCUT("script_text_editor/paste"), EDIT_PASTE);
+ edit_menu->get_popup()->add_shortcut(ED_GET_SHORTCUT("script_text_editor/paste_original_indent"), EDIT_PASTE_ORIGINAL_INDENT);
edit_menu->get_popup()->add_separator();
edit_menu->get_popup()->add_shortcut(ED_GET_SHORTCUT("script_text_editor/select_all"), EDIT_SELECT_ALL);
edit_menu->get_popup()->add_separator();
@@ -1843,6 +1857,7 @@ void ScriptTextEditor::register_editor() {
ED_SHORTCUT("script_text_editor/cut", TTR("Cut"), KEY_MASK_CMD | KEY_X);
ED_SHORTCUT("script_text_editor/copy", TTR("Copy"), KEY_MASK_CMD | KEY_C);
ED_SHORTCUT("script_text_editor/paste", TTR("Paste"), KEY_MASK_CMD | KEY_V);
+ ED_SHORTCUT("script_text_editor/paste_original_indent", TTR("Paste with Original Indent"), KEY_MASK_CMD | KEY_MASK_SHIFT | KEY_V);
ED_SHORTCUT("script_text_editor/select_all", TTR("Select All"), KEY_MASK_CMD | KEY_A);
ED_SHORTCUT("script_text_editor/move_up", TTR("Move Up"), KEY_MASK_ALT | KEY_UP);
ED_SHORTCUT("script_text_editor/move_down", TTR("Move Down"), KEY_MASK_ALT | KEY_DOWN);
diff --git a/editor/plugins/script_text_editor.h b/editor/plugins/script_text_editor.h
index 4dbade472..a01a41bd5 100644
--- a/editor/plugins/script_text_editor.h
+++ b/editor/plugins/script_text_editor.h
@@ -96,6 +96,7 @@ class ScriptTextEditor : public ScriptEditorBase {
} colors_cache;
bool theme_loaded;
+ bool use_space_indentation;
enum {
EDIT_UNDO,
@@ -103,6 +104,7 @@ class ScriptTextEditor : public ScriptEditorBase {
EDIT_CUT,
EDIT_COPY,
EDIT_PASTE,
+ EDIT_PASTE_ORIGINAL_INDENT,
EDIT_SELECT_ALL,
EDIT_COMPLETE,
EDIT_AUTO_INDENT,
|
It uses the indentation type and width configured in the editor settings. There is a new "Paste with Original Indent" context menu option to bypass this conversion. Implementation reuses CodeEditor's `convert_indent*` methods, adding optional arguments for from and to line indices.
akien-mga
force-pushed
the
convert-indent-on-paste
branch
from
7413052
to
c6fe753
Compare
I pushed an update commit which implements this properly, adding optional arguments to the
That's still unsolved though, and needs to be before this can be merged. I guess it's not Related to #31183. |
There was a problem hiding this comment.
Besides the one comment below it looks okay to me. I don't know if it would be better calling it "paste unformatted" rather then "paste original indent", to make it easier to potentially add more functionally such as trimming whitespace?
Might also be worth adding it to the shader and text editors.
As for the shortcut, the easiest way is to handle it before text_edit by processing it in inside of _input (This was done for a few shortcuts inside of CodeTextEditor). Otherwise you'd have to move everything into text_edit (see #12164) or have it emit a signal on paste and handle it from there.
Alternatively, as you say have build a way to disable it, but that's way out of scope.
| @@ -1661,6 +1675,7 @@ void ScriptTextEditor::_make_context_menu(bool p_selection, bool p_color, bool p | |||
| ScriptTextEditor::ScriptTextEditor() { | |||
|
|
|||
| theme_loaded = false; | |||
| use_space_indentation = EditorSettings::get_singleton()->get("text_editor/indent/type"); // Tabs 0, spaces 1 | |||
There was a problem hiding this comment.
Needs to be set under update_settings else it won't update if the setting is changed.
akien-mga
added
the
cherrypick:3.x
|
@akien-mga What's the status of this PR? It needs to be rebased. |
|
I'm struggling to find time to work on this, so closing for now. It's salvageable, anyone feel free to pick it up and adapt it to the 4.0 script editor architecture while addressing the above feedback. |
akien-mga
added
salvageable
and removed
cherrypick:3.x
This is implemented as an editor setting (opt-out), and only for the
script editor (though it's implemented in TextEdit which handles the
paste).
Based on the indent type and size from the editor settings, it will
convert tabs to the expected number of spaces, or spaces to tabs. In
the latter case, it aims to guess the number of spaces used as indent
size in the clipboard.
Fixes #19285.
The implementation is not perfect yet, and likely a bit naive, as my knowledge of the String API is a bit rusty and I didn't pause long to think about what would be the most efficient algorithm. Any feedback/suggestions are welcome.
I've noticed a slight issue with the selection cursor when doing undo, but haven't investigated yet.
Edit: Doesn't seem related to this PR, the same bug happens with the option disabled.
I would also have liked the reformatting to happen as a second undoable complex operation (so paste first without changes, then apply changes, and you undo in two steps), but I'm not sure how to do it easily.