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 triple clicking empty line and dragging #5068

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

otomist
Copy link

@otomist otomist commented Jan 14, 2025

Fixes core issue #4957. Adds a bool to SelectLine allowing the selectLine function to select lines that are empty. When starting a triple selection on an empty line the initial selectLine returns null because we don't see any characters, in this case we rerun selectLine but short circuit with the allow_empty_lines. We need to run selectLine with out allowing empty lines once because if there are characters on the line we don't want to select empty space.

@otomist otomist changed the title add and use flag for selecting empty lines in the selectLine function Fix triple clicking empty line and dragging Jan 14, 2025
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

The approach looks good, some requests.

@@ -2215,6 +2215,7 @@ pub const SelectLine = struct {
/// state changing a boundary. State changing is ANY state
/// change.
semantic_prompt_boundary: bool = true,
allow_empty_lines: bool = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs docs

// If we allow empty lines, we don't need to do any further checks.
if (opts.allow_empty_lines) {
return Selection.init(start_pin, end_pin, false);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be unit tested. Everything in Screen should be unit tested (for historical reasons some parts aren't but going forward we should be adding it it all)

// that had no data, in this case recall selectLine with allow_empty_lines.
if (sel_ == null) {
sel_ = screen.selectLine(.{ .pin = click_pin, .allow_empty_lines = true });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a labeled block to put all of this (line 3570 to here) in a const assignment.

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

Successfully merging this pull request may close these issues.

2 participants