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 moving text in script editor with no selected character on last line #54392

Closed
wants to merge 1 commit into from

Conversation

jmb462
Copy link
Contributor

@jmb462 jmb462 commented Oct 29, 2021

Fixes #54362

Issue

In script editor, when moving selection with alt+up/down, an extra line at the end or at the beginning can be moved.
It happens when selection begins at the end of a line or finishes at the beginning of a line (with no character of those lines selected).

Fix proposal

Check if we are in such a case and prevent those line to move with the selection.
(I also rename a variable from_col to from_column for consistency reason.)

Before

You can see that in some case a line is moved whereas it shouldn't happen.
move_bug

After

Behaviour is the expected one (as in Visual Studio / VS Code)
move_fixed

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.

Tested out other editors seems as though we should only be skipping the last line if to_col == 0. As it still moves the line if the from_col is at the end of the text.

@jmb462
Copy link
Contributor Author

jmb462 commented Nov 3, 2021

Tested out other editors seems as though we should only be skipping the last line if to_col == 0. As it still moves the line if the from_col is at the end of the text.

After checking with VisualStudio, you're right. But there is a visual difference :
image
In VS Studio, VSCode and SublimeText, you see that the carriage return is selected so the selection on that line is not empty.
In godot editor, nothing seems to be selected at the end of the line.

@jmb462 jmb462 changed the title Fix moving text in script editor with no selected character on first or last line Fix moving text in script editor with no selected character on last line Nov 3, 2021
@jmb462 jmb462 requested a review from Paulb23 November 6, 2021 10:10
@@ -1216,6 +1216,11 @@ void CodeTextEditor::move_lines_up() {
return;
}

// Prevent moving last line with no character selected
if (to_column == 0 && i == to_line) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we be taking one from to_line rather then skipping, otherwise below this loop, it looks like to_line_up will be off by one?

@mhilbrunner mhilbrunner modified the milestones: 4.0, 4.x Aug 25, 2022
@aXu-AP
Copy link
Contributor

aXu-AP commented Jan 28, 2023

@jmb462 are you going to continue the work on this? If not, I think I could finish it. This problem has been my biggest issue with script editor since forever, and now I'm at last capable of fixing it (I believe).

@jmb462
Copy link
Contributor Author

jmb462 commented Jan 28, 2023

@jmb462 are you going to continue the work on this? If not, I think I could finish it. This problem has been my biggest issue with script editor since forever, and now I'm at last capable of fixing it (I believe).

Feel free to work on this :)

@aXu-AP
Copy link
Contributor

aXu-AP commented Jan 28, 2023

Thanks, I'll give it a shot :)

@YuriSizov
Copy link
Contributor

Superseded by #72672.

@YuriSizov YuriSizov closed this Feb 3, 2023
@akien-mga akien-mga modified the milestones: 4.x, 4.0 Feb 6, 2023
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.

CodeTextEditor.move_lines_up moves an extra line when the selection ends at new line
7 participants