feat(commands): shrink_selection #1340
Conversation
matoous
changed the title
|
Showcase: Screen.Recording.2021-12-23.at.11.01.31.mov |
matoous
changed the title
|
@archseer I refactored the |
|
I am also wondering about the keybindings. @archseer suggested |
|
I think I would prefer the latter (repeating with the dot). Similar to how |
|
|
|
Oh sorry, I meant Alt-. which repeats the last movement |
|
Added default key mapping to |
helix-core/src/selection.rs
Outdated
|
|
||
| pub fn contains(&self, other: &Selection) -> bool { | ||
| self.iter() | ||
| .zip(other.iter()) |
There was a problem hiding this comment.
Hmm. This won't work correctly if I have three selection ranges and then I call contains with two selection ranges that are a subset of the original selection
There was a problem hiding this comment.
Yeah, you are right. I reworked the method but I am still hesitant whether the check makes sense at all. I think just moving the cursor should disable the shrink_selection so that we are safe, but I guess we can adjust that later if there are any complaints?
There was a problem hiding this comment.
I was pondering this. I think just checking containment will do the right thing when the user flow is expand, expand, shrink, shrink, etc, but you could also have the following sequence
fn foo() {
let a = 1;
[a]
}expand
fn foo() [{
let a = 1;
a
}]move cursor around and back inside the function
fn foo() {
[] let a = 1;
a
}select entire file
[
...
fn foo() {
let a = 1;
a
}
...
]shrink selection
fn foo() {
let a = 1;
[a]
}It might not matter much in practice. But I guess we just have to decide how strict we want to be.
|
I think this shouldn't necessarily be a blocker, but one thing we could do if there haven't been any expands before shrinks is just select the first child node. |
I thought that was already the behavior? I proposed that before: when we run out of selections on the stack it should select the first child instead |
Well, if I'm not mistaken, the current PR does not do anything unless there was previously an expand to populate the selection history. |
Yes, this is the case. |
helix-term/src/commands.rs
Outdated
| doc.set_selection(view.id, prev_selection); | ||
| } else { | ||
| // clear existing selection as they can't be shrinked to anyway | ||
| view.object_selections.clear() |
There was a problem hiding this comment.
Can you add that behavior in here? It should call object::shrink_selection and just look for the first descendant_for_byte_range
There was a problem hiding this comment.
Done, can you please check whether I understood the requirement correctly?
Add `shrink_selection` command that can be used to shrink previously expanded selection. To make `shrink_selection` work it was necessary to add selection history to the Document since we want to shrink the selection towards the syntax tree node that was initially selected. Selection history is cleared any time the user changes selection other way than by `expand_selection`. This ensures that we don't get some funky edge cases when user calls `shrink_selection`. Related: #1328
Add
shrink_selectioncommand that can be used to shrink previously expanded selection.To make
shrink_selectionwork it was necessary to add selection history to the Document since we want to shrink the selection towards the syntax tree node that was initially selected.Selection history is cleared any time the user changes selection other way than by
expand_selection. This ensures that we don't get some funky edge cases when user callsshrink_selection.Related: #1328