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

Deselect multi caret when alt clicking on it #80956

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

MarcusElg
Copy link
Contributor

Fixes #80895

If a caret can't be added it checks if it already exists and then removes it. So now it works similarly to vscode when alt clicking once adds a caret, and alt clicking again removes it.

It does loop through the carets twice but preventing that would require changing the return value of add_caret and therefore break compatibility. The amount of carets are always quite low so the performance impact should be neglectable.

Copy link
Contributor

@MewPurPur MewPurPur left a comment

Choose a reason for hiding this comment

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

I like this implementation, in all honesty I wonder why this wasn't part of the initial implementation.

scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

LGTM, the comment punctuation needs to be fixed though

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master 6758a7f), it mostly works as expected.

However, with this PR, unlike in VS Code, it's not possible to deselect a character/word selection by holding Alt and clicking the selection or the caret:

simplescreenrecorder-2023-08-25_00.08.55.mp4

@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Aug 25, 2023
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.

LGTM, would be nice to add a unit test :)

@MarcusElg
Copy link
Contributor Author

@Calinou fixed, you can now deselect by clicking its selection

scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
scene/gui/text_edit.cpp Show resolved Hide resolved
@akien-mga akien-mga merged commit afd0103 into godotengine:master Sep 11, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@MarcusElg MarcusElg deleted the deselectmulticaret branch September 11, 2023 14:47
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.

Multi carets cannot be deselected
8 participants