Skip to content
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 right click in selection of additional caret #76472

Merged
merged 1 commit into from
May 8, 2023

Conversation

jmb462
Copy link
Contributor

@jmb462 jmb462 commented Apr 26, 2023

Fix #76417

Issue description

Right click in the selection of any secondary caret discard all selections and show context menu has is there was no selection.

Investigations

In ScriptTextEditor::_text_edit_gui_input(const Ref<InputEvent> &ev) multicaret is not managed when checking if click was in a selection. It only check the main caret selection. If it's outside the main caret selection, it discards all secondary caret and all selections.

Fix proposal

(modified version after taking comments into account)

Managed multicarret in ScriptTextEditor::_text_edit_gui_input(const Ref<InputEvent> &ev) : iterate throw each caret selection to check if right click is in any of the caret selections. Keep all selection.

Same modification in void TextEdit::gui_input(const Ref<InputEvent> &p_gui_input) : only one caret selection was checked. Iterates throw all selections and keep it all.

Before

mutlicaret

After

fixed

@ajreckof
Copy link
Member

Is there a particular reason we should only keep one of the selection and not all of them?

@jmb462
Copy link
Contributor Author

jmb462 commented Apr 29, 2023

Is there a particular reason we should only keep one of the selection and not all of them?

In VS Code, we can copy multiselection and selections are each pasted on a different line.
So maybe we could also keep all selections and do the same.

(edit : Done)

@ajreckof
Copy link
Member

Is there a particular reason we should only keep one of the selection and not all of them?

In VS Code, we can copy multiselection and selections are each pasted on a different line. So maybe we could also keep all selections and do the same.

Your coment made me realise that it is already the case for keyboard shortcut and therefore it would be even stranger not to do it for right click.

@jmb462 jmb462 marked this pull request as draft April 29, 2023 20:22
@jmb462
Copy link
Contributor Author

jmb462 commented Apr 30, 2023

Thanks to @ajreckof comments, it clearly appears that we have to keep all carets when right clicking on a selection.

I've modified this PR in this way.

@jmb462 jmb462 force-pushed the multicarets_selection_popup branch from 79da505 to bf7d62c Compare April 30, 2023 16:35
@jmb462 jmb462 marked this pull request as ready for review April 30, 2023 17:27
Copy link
Member

@Paulb23 Paulb23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find and fix!

Couple small notes inline

scene/gui/text_edit.cpp Show resolved Hide resolved
scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
editor/plugins/script_text_editor.cpp Show resolved Hide resolved
@jmb462 jmb462 force-pushed the multicarets_selection_popup branch from bf7d62c to 5c06c03 Compare May 7, 2023 22:03
@akien-mga akien-mga merged commit 32fbba4 into godotengine:master May 8, 2023
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 4.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistant behavior when right clicking in a multiple selection
5 participants