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

LSP-server slow autocomplete response jj conflict #11009

Closed
1 task done
d1y opened this issue Apr 25, 2024 · 7 comments
Closed
1 task done

LSP-server slow autocomplete response jj conflict #11009

d1y opened this issue Apr 25, 2024 · 7 comments
Labels
bug [core label] language server An umbrella label for all language servers language An umbrella label for all programming languages syntax behaviors vim

Comments

@d1y
Copy link
Contributor

d1y commented Apr 25, 2024

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

My custom keymap

  {
    "context": "Editor && vim_mode == insert",
    "bindings": {
      "j j": "vim::NormalBefore"
    }
  }

When the return is slow, triggering jj will conflict and will be directly input into the buffer. (like jjkjkjjkkjxdfjk)

Slow? #5166 (comment)

/cc @mrnugget @ConradIrwin

Screen.Recording.2024-04-26.at.03.10.02.mov

The VScode faster 👀, and no incorrect inference prompts in {}

Screen.Recording.2024-04-26.at.03.18.41.mov

Environment

Zed: v0.133.2 (Zed Preview)
OS: macOS 14.4.1
Memory: 16 GiB
Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

No response

@d1y d1y added admin read Pending admin review bug [core label] triage Maintainer needs to classify the issue labels Apr 25, 2024
@d1y
Copy link
Contributor Author

d1y commented Apr 25, 2024

I don't know if I expressed myself clearly

<script setup lang="ts">
import { ref, reactive } from 'vue'
const value = re
//              ^ <-- Cursor is here
//              ^ <-- Assuming that waiting for the lsp to return is too slow here and I want to enter it manually
//              ^ <-- Trigger (jj)`jjkjjaxv` enter insert mode it won't work
</script>

@ConradIrwin
Copy link
Member

Thanks for the report, makes sense, and I will see if I can reproduce.

@JosephTLyons JosephTLyons added vim language An umbrella label for all programming languages syntax behaviors language server An umbrella label for all language servers and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Apr 26, 2024
@ConradIrwin
Copy link
Member

@d1y I heard that this might be resolved.

I think the likely problem (before) #12630 was that when the completion request returned we would change the keymap context; and when we change the context that invalidates any multi-key bindings.

I'm going to close this, but please re-open if you can still reproduce.

I am not sure why "j j": ["vim::SwitchMode", "Normal"] would have stopped working. If I have that in my key bindings it still seems to work. Do you have anything else going on that might cause problems?

@d1y
Copy link
Contributor Author

d1y commented Jun 10, 2024

When I input quickly, there is still a problem. GIF:

fxxx

const x = falsejjkj
         // Fast input `false` and `j j` And `k` 
         // There will be this problem

Compared to VScode is silky and smooth 🤔

Screen.Recording.2024-06-11.at.03.28.05.mov

👇🏻👇🏻👇🏻The following questions can actually be deleted! Solving the binding of menu 👇🏻👇🏻👇🏻

{
    "context": "Editor && vim_mode == insert",
    "bindings": {
      "j j": "vim::NormalBefore"
    }
}

I want to re-open this question because I still have issues when using it in real life
I provide a repo for reproducing the problem: https://github.com/sb-child/lsp

[
  {
    "context": "Editor && vim_mode == insert && !menu",
    "bindings": {
      "j j": "vim::NormalBefore"
    }
  }
]
Screen.Recording.2024-06-10.at.20.22.57.mov

I use zed-nightly

Zed: v0.140.0 (Zed Nightly e829a8c)
OS: macOS 14.5.0
Memory: 16 GiB
Architecture: aarch64

@Moshyfawn Moshyfawn reopened this Jun 10, 2024
ConradIrwin added a commit that referenced this issue Jul 22, 2024
Simplify key dispatch code.

Previously we would maintain a cache of key matchers for each context
that
would store the pending input. For the last while we've also stored the
typed prefix on the window. This is redundant, we only need one copy, so
now
it's just stored on the window, which lets us avoid the boilerplate of
keeping
all the matchers in sync.

This stops us from losing multikey bindings when the context on a node
changes
(#11009) (though we still interrupt multikey bindings if the focus
changes).

While in the code, I fixed up a few other things with multi-key bindings
that
were causing problems:

Previously we assumed that all multi-key bindings took precedence over
any
single-key binding, now this is done such that if a user binds a
single-key
binding, it will take precedence over all system-defined multi-key
bindings
(irrespective of the depth in the context tree). This was a common cause
of
confusion for new users trying to bind to `cmd-k` or `ctrl-w` in vim
mode
(#13543).

Previously after a pending multi-key keystroke failed to match, we would
drop
the prefix if it was an input event. Now we correctly replay it
(#14725).

Release Notes:

- Fixed multi-key shortcuts not working across completion menu changes
([#11009](#11009))
- Fixed multi-key shortcuts discarding earlier input
([#14445](#14445))
- vim: Fixed `jk` binding preventing you from repeating `j`
([#14725](#14725))
- vim: Fixed `escape` in normal mode to also clear the selected
register.
- Fixed key maps so user-defined mappings take precedence over builtin
multi-key mappings
([#13543](#13543))
- Fixed a bug where overridden shortcuts would still show in the Command
Palette
@d1y
Copy link
Contributor Author

d1y commented Jul 22, 2024

Hi @ConradIrwin
If #14942 fixes this issue, I just want to report:
After using zed night, I found that there are still problems when I quickly input 'jjk'

Screen.Recording.2024-07-23.at.02.59.11.mov

@ConradIrwin
Copy link
Member

@d1y I am struggling to reproduce this after #14942.

Trying to look at this video, I am also a bit confused. It seems like your keyboard overlay is missing some keystrokes. Can you please let me know what you typed exactly to get flajgjk to show up?

For me on Zed Preview, I can reproduce a problem starting from:
Screenshot 2024-07-22 at 15 53 09

I then type o f j j as fast as possible. If I manage to get the first j before the autocomplete shows up then I can reproduce this bug.

On Zed Nightly, I cannot reproduce the bug with this technique.

@d1y
Copy link
Contributor Author

d1y commented Jul 22, 2024

@ConradIrwin I'm sorry for bothering you
I think you're right!
I have used all three editors and found that the behavior is consistent

I think I just accidentally touched my fingernails because they have grown too long, so I have trimmed them now 🤡

Screen.Recording.2024-07-23.at.06.27.34.mov

@d1y d1y closed this as completed Jul 22, 2024
CharlesChen0823 pushed a commit to CharlesChen0823/zed that referenced this issue Jul 29, 2024
Simplify key dispatch code.

Previously we would maintain a cache of key matchers for each context
that
would store the pending input. For the last while we've also stored the
typed prefix on the window. This is redundant, we only need one copy, so
now
it's just stored on the window, which lets us avoid the boilerplate of
keeping
all the matchers in sync.

This stops us from losing multikey bindings when the context on a node
changes
(zed-industries#11009) (though we still interrupt multikey bindings if the focus
changes).

While in the code, I fixed up a few other things with multi-key bindings
that
were causing problems:

Previously we assumed that all multi-key bindings took precedence over
any
single-key binding, now this is done such that if a user binds a
single-key
binding, it will take precedence over all system-defined multi-key
bindings
(irrespective of the depth in the context tree). This was a common cause
of
confusion for new users trying to bind to `cmd-k` or `ctrl-w` in vim
mode
(zed-industries#13543).

Previously after a pending multi-key keystroke failed to match, we would
drop
the prefix if it was an input event. Now we correctly replay it
(zed-industries#14725).

Release Notes:

- Fixed multi-key shortcuts not working across completion menu changes
([zed-industries#11009](zed-industries#11009))
- Fixed multi-key shortcuts discarding earlier input
([zed-industries#14445](zed-industries#14445))
- vim: Fixed `jk` binding preventing you from repeating `j`
([zed-industries#14725](zed-industries#14725))
- vim: Fixed `escape` in normal mode to also clear the selected
register.
- Fixed key maps so user-defined mappings take precedence over builtin
multi-key mappings
([zed-industries#13543](zed-industries#13543))
- Fixed a bug where overridden shortcuts would still show in the Command
Palette
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [core label] language server An umbrella label for all language servers language An umbrella label for all programming languages syntax behaviors vim
Projects
None yet
Development

No branches or pull requests

4 participants