Skip to content

fix(suggestion): Trigger suggestion on InsertEnter#318

Merged
zbirenbaum merged 1 commit intozbirenbaum:masterfrom
jsongerber:master
Feb 10, 2025
Merged

fix(suggestion): Trigger suggestion on InsertEnter#318
zbirenbaum merged 1 commit intozbirenbaum:masterfrom
jsongerber:master

Conversation

@jsongerber
Copy link
Copy Markdown
Contributor

This pull request make that the suggestion get triggered when entering insert mode.
It was not working because both InsertEnter and CursorMovedI would fire at the same time and the timer variable would be messed up in the debounce function like this:

  • InsertEnter is called, copilot.timer is set to timer id 1
  • CursorMovedI is called, copilot.timer is set to timer id 2
  • Debounce function 1 is called, timer is 1, copilot.timer is 2, early return, copilot.timer is set to nil
  • Debounce function 2 is called, timer is 2, copilot.timer is nil, early return

This pull requests fixes this, I'm testing this since yesterday and it's working fine, but I don't know the codebase very well so I'm hoping I'm not breaking something.

Also the debounce function doesn't seem to be working (with or without this pull request), it works only as a timeout, does this need to be fixed or is this the intended behavior?

@folke
Copy link
Copy Markdown

folke commented Jan 31, 2025

@zbirenbaum any chance we can have this merged?
This also fixes suggestions not working when doing o or O.

@folke
Copy link
Copy Markdown

folke commented Jan 31, 2025

Nevermind, this PR seems to break in a lot of other ways

@jsongerber
Copy link
Copy Markdown
Contributor Author

Nevermind, this PR seems to break in a lot of other ways

Can you give me some examples of bugs this introduce? I'm using this PR with no issue, at least for suggestions (I'm not using panels and other features).
Maybe if I can fix thoses bugs this PR could be merged. Or I can close otherwise.
Thank you!

@folke
Copy link
Copy Markdown

folke commented Jan 31, 2025

Haven't looked into it, but I started getting errors where suggestions tried to show when it was not allowed to change the buffer

@jsongerber
Copy link
Copy Markdown
Contributor Author

Thank you for your response. I rebased my fork and will use this PR with the latest changes for the next few days to see if I can reproduce.
Feel free to tag me if you or anyone using this PR has more information to help fix the issue.

@folke
Copy link
Copy Markdown

folke commented Jan 31, 2025

Great, I'm also using the update branch. Will let you know when I experience issues but seems to work fine now.
ty!

@bulletmark
Copy link
Copy Markdown

I also tried that updated commit and it doesn't work the first time I open a new line in a newly opened file. If I hit escape, and then open the line again it works. Doesn't matter if I wait for a while. I.e. only works 2nd and subsequent times I open a new line. I am doing the test I described here in both a Python file and a shell script.

@jsongerber
Copy link
Copy Markdown
Contributor Author

Thank you @bulletmark for your feedback, I will check this weekend and see if I can reproduce and fix

@jsongerber
Copy link
Copy Markdown
Contributor Author

@bulletmark I could not reproduce the error, here is a screen recording showing Copilot working using O directly after opening the buffer with the Fibonacci example (it's slow to appear as I'm on the train with slow connection, but it eventually works):

Screen.Recording.2025-01-31.at.18.14.45.mov

I could reproduce the buggy behavior using lazy loading, and removing event = 'InsertEnter' fixes the issue (which LazyVim seems to be using).
I don't know if the problem should be fixed in this plugin or if it's normal behavior.

I tried to use lazy loading in my config and manually trigger a suggestion when entering insert mode the first time, but the server is not ready yet RPC[Error] code_name = ServerNotInitialized, message = "Agent service not initialized.". I could not find a reliable way to get the status of the server. Using a timer should work but is hacky.

@folke
Copy link
Copy Markdown

folke commented Jan 31, 2025

Great. Just changed it in LAzyVim and works as expected indeed.
And that annoyoing error is gone. I've been seeing it since a while, but never took the effort to further investigate :)

@apaydev
Copy link
Copy Markdown

apaydev commented Jan 31, 2025

So, is there an estimated date when this will be implemented in LazyVim? I just opened today nvim to work on some stuff and just came across this exact issue hahaha, and modifying my config files does not seem to change the behavior.

@folke
Copy link
Copy Markdown

folke commented Jan 31, 2025

@ADPenrose what do you mean? LazyVim != copilot.lua

@folke
Copy link
Copy Markdown

folke commented Jan 31, 2025

Or you mean that LSP notification? That's already solved on main

@folke
Copy link
Copy Markdown

folke commented Feb 7, 2025

Have been using this PR for a while now without issues, so would be great if this could get merged.

Thank you!

@bulletmark
Copy link
Copy Markdown

@folke, does LazyVim have a feature where I can merge this patch to my local installation? I see this but can't find any docs on how to use it?

@zbirenbaum
Copy link
Copy Markdown
Owner

This looks good to me, merging. Thanks for the fix @jsongerber

@zbirenbaum zbirenbaum merged commit 30321e3 into zbirenbaum:master Feb 10, 2025
@bulletmark
Copy link
Copy Markdown

Yay, updated LazyVim which picked up this change. Then tested against my two fibanacci examples (python and shell) and it worked perfectly! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants