helix-mode: introduce HelixRange selection type (no wiring)#1061
Conversation
|
This is a pretty small changeset with many tests (thanks). It looks good to me but I'd like to get @schlich's input on it before we land it. Thanks! |
|
I'll take a look either tonight or this weekend! |
|
@schlich Have you had time to review yet? Sorry to nag. |
|
Hey @fdncred and @kronberger-droid et al! Sorry for the delay and silence, time has really been getting away from me. I'm just catching up, haven't looked at other threads/issues in here yet. And sorry for the wall of text paragraph , currently on mobile... I was actually able to go over this one and was in the process of putting some feedback down and obtain my own understanding of the follow up issues that were referenced, of course some distractions came up and I wasn't able to submit a thoughtful review. From a high level the abstractions make sense although I was trying to piece together how they relate to the missing word boundary behavior we are tackling. I think that in the interest of moving things along its pretty safe to merge these objects so that we have them available, I'm not convinced how they fit in the big picture but it seems @kronberger-droid knows where we are going. Test logic seemed solid but again not the most thorough evaluation. Will have much more time to work on reedline this week |
|
Thanks @schlich for getting back with us. Are you ready to land this @kronberger-droid ? |
|
Yeah from my side its cleared. I am pretty confident that without this abstraction helix mode will be a mess. Next steps would be to continue the dicussion i started in the helix mode PR. since it will determine how to go forward. |
|
Thanks y'all. I'm trying to keep things moving so y'all aren't waiting on the nushell team. |
Summary
Introduces
HelixRange, a anchor/head selection range type similar to helix-core, along with aDirectionenum. The type has no callers yet and is gated behind#![allow(dead_code)]until future PRs.Motivation
Part of #960, and follow-up to #1039.
Reedline already has selection state via
Editor::selection_anchorplus the insertion point. That model fits vi's visual mode but doesn't carry two things helix needs:selection_anchorexists only when the user has actively started a selection in vi.Rather than retrofit
selection_anchorto carry direction, this PR adds a helix-local type that encodes both properties.Why direction matters
Behaviors that depend on this:
What's included
HelixRange { anchor, head }with gap-indexing semantics (left-inclusive, right-exclusive, regardless of anchor/head ordering)HelixRangeDirection::{Forward, Backward}enumWhat's deliberately deferred
Selection: reedline is single line.Adaptations from helix-core
RopeSlice&str(future PRs)String, notRopeTests
All 32 tests pass.
The module is
pub(super)and gated behind thehelixfeature flag, thus no behavior change possible.