helix-editor / helix

A post-modern modal text editor.
https://helix-editor.com
Mozilla Public License 2.0
33.76k stars 2.51k forks source link

Weird selection with multiline snippet #6245

Closed Urgau closed 1 year ago

Urgau commented 1 year ago

Summary

When using rust-analyzer and and typing fn inside an impl [..] for [..] block the selection is span across multiple lines.

image

cc @pascalkuthe

Reproduction Steps

I tried this:

  1. [..]/target/debug/hx src/lib.rs
  2. Add a dummy Rust struct
  3. Add an impl Display for [..] (don't forget to import Display)
  4. Type fn inside the impl block
  5. Go to the fn fmt completion snippet

I expected this to happen:

Only have a single point cursor inside the function.

Instead, this happened:

The cursor is correctly placed but it's selection is spanning across the line.

Helix log

~/.cache/helix/helix.log No relevant by it's self. Checked myself.

Platform

Linux

Terminal Emulator

Alacritty

Helix Version

22.12-382-gd63e570e

pascalkuthe commented 1 year ago

This is om purpose. In helix the selection has two ends: head and anchor. head. The anchor always stays in place whilehead moves while editing. In insertmode you insert before the cursor usually si the selection appears to stay fixed. But with a snippet we movehead` to the tabstop and therefore increase the size of the selection.

An example that makes it a little clearer (| represent start/end of selection)

fn|

fn bar{}||

If you press i and select the snippet then the selection changes as follows:

fn |() {

}

fn bar{}||

Here the first tabstop was the function name so head was moved to the right by 1 but anchor stays in place. Your case is a bit more subkt debut essentially there anchor is on the right side of the space after fn which is not removed by the snippet.

Basically tabstops inky move the cursor not the entire selection

Urgau commented 1 year ago

I'm not sure I understood what you were trying to say.

If I understand correctly when I'm typing fn the cursor is just after the n and so when applying the snippet the tabstop inside the function is where the head is going to be and the anchor didn't move that is anchor is after the end of the function, right ?

If so why does is only when with this snippet and not any other one?

Note: I didn't mention it but I didn't have any selection prior to typing fn.

pascalkuthe commented 1 year ago

It does work like this for every other snippet too but only if you use insert mode (i). Appendmode (a, c) work differently. They keep the anchor on the left side and extend to the right. Again an a bit less obvious example:

If you are at

|Rope::| 

and then hit a the selection becomes

|Rope:: |

If you select the char_to_line(..) snipppet from the completions the selection becomes:

|Rope::char_to_line(,|)

Note that the last selection char is the cursor, so the above looks a bit confusing. If you type foo this would become

|Rope::char_to_line(foo,|)

and if you exit appendmode it would become (cursor removed):

|Rope::char_to_line(foo|,)

Altough there is currently one inconsistency that you might have gotten confused by: If the cursor is fully inside the replaced text then the cursor is always moved to the tabstop as a point selection.

For example if you have the following selection:

Rope:|:|

and append:

Rope:|: |

and select the line_to_char(..) snippet you get the following selection (so text is inserted before the ,):

Rope::line_to_char(|,|)

but to be consistent the seleciton should be:

Rope:|:line_to_char(,|)

The reason it works this way is that the completion rust analyzer sends replaces :: with ::line_to_char(,). We currently do not look "inside" the completions to tell if there is anything unchanged. We probably should compute such compute such a diff to make that beahve consistenly but this problem is an existing bug and occurs for normal completions too.

For example

Rope::lin|e_to|

if you append here and select the line_to_char completion (the non-snippet version) you get the following selection:

Rope::line_to_char| |

instead of

Rope::lin|e_to_char |

I think the right think to do would be to strip the common prefix from compeltions when applying them. We actually had something similar in the codebase for cases where LS didn't send a range but that was buggy and not spec compliant. What I am thinking is that we should always apply the edit the same but check (for each cursor) if there is a common prefix and fix that.

Urgau commented 1 year ago

Okay, I think I now better understand what is going on. I'm still a little surprised by the behavior, especially since it only trigger in (very?) rare situations.

Anyway, since it's working as intended, closing it.