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

Auto completing deletes character cursor is currently on. #5761

Closed
dogunbound opened this issue Feb 1, 2023 · 17 comments
Closed

Auto completing deletes character cursor is currently on. #5761

dogunbound opened this issue Feb 1, 2023 · 17 comments
Assignees
Labels
C-bug Category: This is a bug upstream

Comments

@dogunbound
Copy link

Summary

A video is better at explaining it:

2023-01-31.19-09-38.mp4

Reproduction Steps

Open a rust project.

In main create a local variable then do:
type:
dbg!(
Then try and autofill the local variable

The opening parenthesis should disappear.

Helix log

No log gets outputted when this issue occurs.

My config:

theme="dracula"

[editor]
mouse=false
completion-trigger-len=1
idle-timeout=0
auto-save=true
scrolloff=3

[keys.select]
 k = ["ensure_selections_forward", "flip_selections", "extend_line_up"]```

### Platform

Linux

### Terminal Emulator

Konsole

### Helix Version

22.12
@dogunbound dogunbound added the C-bug Category: This is a bug label Feb 1, 2023
@gabydd
Copy link
Member

gabydd commented Feb 1, 2023

Might be helpful to post a relevant snippet of your log with when running helix -vvvv

@CptPotato
Copy link
Contributor

I think this could be resolved by #1819

@pascalkuthe
Copy link
Member

Try setting idle-timeout to a low but more reasonable value like 100 (still way below human reaction time). An idle timeout of 0 is known to cause issues. Although I also think that #1819 might be the real fix to this problem

@dogunbound
Copy link
Author

2023-02-01.17-46-06.mp4

Try setting idle-timeout to a low but more reasonable value like 100 (still way below human reaction time). An idle timeout of 0 is known to cause issues. Although I also think that #1819 might be the real fix to this problem

didn't help

@pascalkuthe
Copy link
Member

Thanks for testing. That's a bit odd I will try to reproduce this locally. These bugs are very hard to reproduce usually but given that you seem to be able to do it reliably (no matter the idle timeout) then maybe I can reproduce this

@dogunbound
Copy link
Author

Might be helpful to post a relevant snippet of your log with when running helix -vvvv

empty log file.

@kirawi
Copy link
Member

kirawi commented Feb 5, 2023

Did you try executing the auto-complete action with logs enabled? The log shouldn't be empty if the LSP is working.

@kirawi kirawi added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 5, 2023
@dogunbound
Copy link
Author

helix.log

@pascalkuthe pascalkuthe removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 3, 2023
@pascalkuthe pascalkuthe self-assigned this Mar 3, 2023
@pascalkuthe
Copy link
Member

This is unrelated to #1819 / #6173 (not fixed by either). I am able to reproduce.

@pascalkuthe
Copy link
Member

I have managed to confirm this is an upstream issue with rust-analyzer. RA seems to always send a range that starts one character too early when completing a macro arg when no text has been typed yet.

Minimal reproducible example:

macro_rules! foo {
    ($val:expr $(,)?) => {
        $val
    };
}

fn main(){
    foo!(); // buggy
    foo!(b); // ok
}

Place the curosor between the brackets of the first foo() press c-x and select any completion (important do not type any text). This will insert the completion and remove the bracket. For the second macro invocation foo!(b) the same bug does not occur. The same issue can reproduced in vscode. Again no text must be typed and instead the completion must be manually invoked with c-space.

Interestingly enough this bug is quite noticeable in helix because helix triggers completion on trigger chars (the ( in this case, which is one of the trigger chars provided by RA [":", ".", "'", "("]) vscode does not trigger completions in this case despite the fact that it's documentation claims that it does

@pascalkuthe
Copy link
Member

Ah it seems that RA prevents showing many completions when the trigger character is a ( as in this case:

https://github.com/rust-lang/rust-analyzer/blob/73e2505cfa8be6838f5151b272f1d24869b2a3d6/crates/ide-completion/src/lib.rs#L158

We currently don't send the trigger character. I think there is actually a lot of room to improve the way helix handles trigger characters. From the LSP spec it actually sounds like the client should treat any word character as a completion trigger too (which is what vscode does):

  The additional characters, beyond the defaults provided by the client (typically
  [a-zA-Z]), that should automatically trigger a completion request. For example
  `.` in JavaScript represents the beginning of an object property or method and is
  thus a good candidate for triggering a completion request.
  
  Most tools trigger a completion request automatically without explicitly
  requesting it using a keyboard shortcut (e.g. Ctrl+Space). Typically they
  do so when the user starts to type an identifier. For example if the user
  types `c` in a JavaScript file code complete will automatically pop up
  present `console` besides others as a completion item. Characters that
  make up identifiers don't need to be listed here.

I know @archseer doesn't like completions popping up instantly so instead of changing the default here we could simply add a config option for that. I believe this is also the correct fix for #5054, #2027 and #2581. VSCode actually does no "on idle" completions at all, it just has these additional trigger chars. So for people that want fast completions that seems like a much better approach (especially because it requires one redraw less and doesn't break other idle timeout related things) .

This bug is orthogonal to that and should be fixed upstream but these might be nice improvements for helix regardless.

@pascalkuthe
Copy link
Member

pascalkuthe commented Mar 4, 2023

Upstream PR submitted: rust-lang/rust-analyzer#14247

@gyreas
Copy link

gyreas commented Mar 4, 2023

This happened to me yesterday (for the first time), using the Hx 22.12

1 similar comment
@gyreas
Copy link

gyreas commented Mar 4, 2023

This happened to me yesterday (for the first time), using the Hx 22.12

@pascalkuthe
Copy link
Member

The upstream PR fixes this issue has been merged. To fix the issue you can build rust analyzer from source for now. The fix will be includes with the next RA release on Monday

@dogunbound
Copy link
Author

This is still happening to me on ra 1.68.1

2023-03-23.23-34-39.mp4

@dogunbound
Copy link
Author

Moved my RA version to nightly. It works now.

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 upstream
Projects
None yet
Development

No branches or pull requests

6 participants