-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add shrink equivalent of extend_to_line_bounds #2450
Add shrink equivalent of extend_to_line_bounds #2450
Conversation
8bfe5ee
to
ac84862
Compare
I fixed the potential for panic. I originally had this so that the shrink would leave the cursor on the character before the EOL, but this led to some interesting bugs where empty lines would be trimmed from the selection even though in spirit the whole line is part of the selection. Including the EOL in the selection resolves this, while also mirroring the behavior of |
I just gave this a try and it looks good: I can't find any edge cases. I think |
doc.set_selection( | ||
view.id, | ||
doc.selection(view.id).clone().transform(|range| { | ||
let text = doc.text(); | ||
|
||
let (start_line, end_line) = range.line_range(text.slice(..)); | ||
|
||
// Do nothing if the selection is within one line to prevent | ||
// conditional logic for the behavior of this command | ||
if start_line == end_line { | ||
return range; | ||
} | ||
|
||
let mut start = text.line_to_char(start_line); | ||
|
||
// line_to_char gives us the start position of the line, so | ||
// we need to get the start position of the next line. In | ||
// the editor, this will correspond to the cursor being on | ||
// the EOL whitespace charactor, which is what we want. | ||
let mut end = text.line_to_char((end_line + 1).min(text.len_lines())); | ||
|
||
if start != range.from() { | ||
start = text.line_to_char((start_line + 1).min(text.len_lines())); | ||
} | ||
|
||
if end != range.to() { | ||
end = text.line_to_char(end_line); | ||
} | ||
|
||
if range.anchor <= range.head { | ||
Range::new(start, end) | ||
} else { | ||
Range::new(end, start) | ||
} | ||
}), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to move this into selection
variable to reduce one layer of nesting.
* Add shrink equivalent of extend_to_line_bounds * Add a check for being past rope end in end position calc * Include the EOL character in calculations * Bind to `A-x` for now * Document new keybind
* Add shrink equivalent of extend_to_line_bounds * Add a check for being past rope end in end position calc * Include the EOL character in calculations * Bind to `A-x` for now * Document new keybind
Implements #2002, with a difference in that using the command when the selection spans only a single line will not result in conditional logic.
I believe there's some holes in the logic that could open to a panic so marking as draft until I can address.