Skip to content
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

Completion-Replacement removes punctuation #12335

Open
OptimisticPeach opened this issue Dec 25, 2024 · 2 comments
Open

Completion-Replacement removes punctuation #12335

OptimisticPeach opened this issue Dec 25, 2024 · 2 comments
Labels
C-bug Category: This is a bug

Comments

@OptimisticPeach
Copy link

Summary

Autocompletion with completion-replacement results in the deletion of punctuation after cursor sometimes.

Reproduction Steps

  1. `hx .
  2. Go to a line in a file that looks like pub fn make_splits(world: &World) -> Vec<Vec<usize>> {} as an example. Make sure to let RA load fully.
  3. Highlight World, cWor<Tab><Enter> to accept autocomplete.

I expected this to happen:

The line in the end remains as described above.

Instead, this happened:

I get the following:

pub fn make_splits(world: &World -> Vec<Vec<usize>> {}

With my cursor positioned after d in World (i.e. between d and ).

Of interest is that typing Wo (as opposed to Wor) doesn't cause it, and typing Worl causes the space after d to be eaten up too, and my cursor to be placed _before the d (i.e. between l and d). Similarly, fully typing World and then accepting the World suggestion causes ...&World> Vec<... to be produced, and Wo to be highlighted with the anchor between & and W.

Helix log

~/.cache/helix/helix.log (Log cleared before running reproduction steps, steps then run and saved, log opened and entire contents pasted below) ``` 2024-12-24T21:34:36.644 helix_view::document [ERROR] Failed to copy metadata on write: The operation completed successfully. (os error 0) ```

Platform

Windows

Terminal Emulator

wezterm 20240203-110809-5046fc22

Installation Method

source

Helix Version

helix 24.7 (0fd4a4a)

@OptimisticPeach OptimisticPeach added the C-bug Category: This is a bug label Dec 25, 2024
@RoloEdits
Copy link
Contributor

This was something I discovered a while ago, and then made a temp fix for and forgot about (😆)

My fix involved changing how the range is calculated:

     fn completion_range(
         text: RopeSlice,
         edit_offset: Option<(i128, i128)>,
         replace_mode: bool,
         cursor: usize,
     ) -> Option<(usize, usize)> {
-        let res = match edit_offset {
-            Some((start_offset, end_offset)) => {
-                let start_offset = cursor as i128 + start_offset;
-                if start_offset < 0 {
-                    return None;
-                }
-                let end_offset = cursor as i128 + end_offset;
-                if end_offset > text.len_chars() as i128 {
-                    return None;
+        let res = if replace_mode {
+            find_completion_range(text, replace_mode, cursor)
+        } else {
+            match edit_offset {
+                Some((start_offset, end_offset)) => {
+                    let start_offset = cursor as i128 + start_offset;
+                    if start_offset < 0 {
+                        return None;
+                    }
+                    let end_offset = cursor as i128 + end_offset;
+                    if end_offset > text.len_chars() as i128 {
+                        return None;
+                    }
+
+                    (start_offset as usize, end_offset as usize)
                 }
-                (start_offset as usize, end_offset as usize)
+                None => find_completion_range(text, replace_mode, cursor),
             }
-            None => find_completion_range(text, replace_mode, cursor),
         };
+
         Some(res)
     }

It checks if its in replace mode and then always calculates the range, ignoring what the lsp or anything else would have sent it. I wasnt able to figure out if the lsp was sending the incorrect range or if helix was doing something with it, so stopped here with it.

I dont know if the maintainers would take this "fix" as its really more of a band-aid.

@pascalkuthe
Copy link
Member

pascalkuthe commented Dec 25, 2024

We will not accept the change above that is not spec compliant and causes issues in other situations.

This seems like a RA bug but you can try #12214 to see if it helps since tgat is the only thing we are missing. You could also try if the same thing reproduces in VSCode (with completion replace enabled)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

3 participants