helix icon indicating copy to clipboard operation
helix copied to clipboard

Add support for target column preservation

Open taupiqueur opened this issue 2 years ago • 2 comments

Reference: https://github.com/mawww/kakoune/issues/64

taupiqueur avatar Sep 25 '22 11:09 taupiqueur

I provide an example to better understand the issue.

hello
hi

The cursor is on o in "hello". j puts the cursor on the EOL besides the i in "hi". Then after pressing hl followed by k the cursor should go back to the o in "hello" instead the cursor goes to the first l in "hello".

ErdoganSeref avatar Sep 28 '22 19:09 ErdoganSeref

@taupiqueur and @ErdoganSeref seem to be talking about two different behaviors here: the linked kakoune issue is for maintaining cursor position WITHOUT using horizontal movement commands (h, l), and @ErdoganSeref's example implies that helix should remember the column number that the cursor was at upon leaving each row, and return to that column upon returning to the row. Right now there is only one column index that gets tracked: horiz in Range (maybe column_index is even a better name?), which allows for the pure vertical movement column index memory, but changes on intentional horizontal movement (i.e, horizontal movement not caused by cursoring vertically to a shorter line). Helix already has the behavior that the kakoune issue is asking for; if it didn't, you would find that starting on the o in hello and hitting jk would put you on the first l in hello. FWIW, I probably wouldn't use the "per row column memory" option myself. The EOL tracking behavior mentioned in the kakoune issue could be neat though.

Carrotstrip avatar Oct 12 '22 20:10 Carrotstrip

Would it make sense tracking these two features in separate issues? It seems to me that implementing the EOL tracking behavior would be an obvious improvement, but the per-row column memory would not be a no-brainer.

marc-thieme avatar Aug 27 '23 14:08 marc-thieme

Also, what do you think about implementing analogous behavior when scrolling at the beginning of the line.

For instance:

    This is inden[t]ed
This is not
         This is again

Now I type gs

    [T]his is indented
This is not
         This is even further

Now I type j

    This is inde[t]nted
[T]his is not
        This is even further

And Type j again

    This is inde[t]nted
This is not
         [T]his is even further

And k would do the opposite

    This is inde[t]nted
[T]his is not
         This is even further

marc-thieme avatar Aug 27 '23 14:08 marc-thieme

As for the implementation, how would one go about implementing that? The last position is stored in the old_visual_position field of the range, right? Would it be sensible to store an enum here, instead? Before:

pub struct Range {
    /// The anchor of the range: the side that doesn't move when extending.
    pub anchor: usize,
    /// The head of the range, moved when extending.
    pub head: usize,
    /// The previous visual offset (softwrapped lines and columns) from
    /// the start of the line
    pub old_visual_position: Option<(u32, u32)>,
}

After:

pub struct Range {
    /// The anchor of the range: the side that doesn't move when extending.
    pub anchor: usize,
    /// The head of the range, moved when extending.
    pub head: usize,
    /// The previous visual offset (softwrapped lines and columns) from
    /// the start of the line
    pub old_visual_position: Option<OldVisualPosition>,
}

enum OldVisualPosition {
    EndOfLine,
    FirstNonBlank,
    Position(u32, u32),
}

marc-thieme avatar Aug 27 '23 15:08 marc-thieme

helix already has the feature described in the original Hakone issue. As long as you only use j/k the cursor always ends up at the same column. I am closing this since that is what this issue was originally about. I don't think we would want to implement the other behaviour described here (and even if we did that should be a separate issue).

Tracking the cursor column during scrolling is a nice improvement and something that someone is already working on IIRC (not that hard to implement). But again a separate issue

pascalkuthe avatar Aug 27 '23 15:08 pascalkuthe