zbirenbaum / copilot-cmp

Lua plugin to turn github copilot into a cmp source
MIT License
1.13k stars 41 forks source link

double prompt #15

Closed winkee01 closed 1 year ago

winkee01 commented 2 years ago

When I hit Enter to select the candidate, it confirms the selection, but it prompts a cmp list again. CleanShot 2022-07-01 at 21 07 54

zbirenbaum commented 2 years ago

Sorry for the delay, I've been pretty busy. I managed to reproduce the menu appearance, but I cant reproduce it actually changing any text on accepting the completion a second time. Whenever I accept the entry after the text exists nothing changes, which is a bit strange.

I'll take a look at this some time over the next couple days and try to get it sorted out.

yngwi commented 2 years ago

I just wanted to chime in and tell you that I have the same problem. I'm not sure about the causes but whenever I get a single copilot completion, using Enter adds the line but the suggestion stays visible and has to be removed by clicking or .

zbirenbaum commented 2 years ago

I think I found the general location of the problem. Removing all of the complex string manipulation and line parsing I do to handle avoiding duplicate lines that already exist in in code, properly indenting, etc causes a (poorly formatted) version of completion to properly appear and then have the menu disappear when I replicate the code in the above video.

There have admittedly been some very minor issues with this code I have noticed regarding last characters not being inserted and I think this is related to that, though I have absolutely no clue how. The code that handles it is somewhat delicate so it may take me a few more days to figure it out. This is likely mostly the fault of upstream stuff regarding completion insertion and menu logic, but it is apparently still fixable on my end, so I will do my best.

If you can manage to find an example of this outside of Rust it would be super helpful to post it, as single completion results in python, lua, and C all appear fine. Writing a similar struct in C functions perfectly, my debug logs show everything functioning properly and I am struggling to find a reason in my code base for this to occur.

winkee01 commented 2 years ago

@zbirenbaum I tested on python, same symptom

https://user-images.githubusercontent.com/49930785/179905560-eacfad48-2348-41a3-92c5-20fcef76e5f4.mp4

zbirenbaum commented 2 years ago

@zbirenbaum I tested on python, same symptom

python_double_prompt.mp4

Now that's very interesting. I can't reproduce that at all. Would you mind posting your cmp config? @yngwi Could you see if you can reproduce the behavior in the python video and post yours as well? It would help to see any commonalities that might lead to this.

winkee01 commented 2 years ago

I basically follow the instructions in the README, and I checked my cmp.lua, can't find anything wrong possible, here is the content in it:

sources = cmp.config.sources({
  { name = 'copilot', priority = 9, max_item_count = 3, group_index = 2},
  { name = 'nvim_lsp', priority = 8, },
  { name = 'luasnip' },
  { name = 'path' },
  { name = "spell" }
  ...
})

and I use packer to manage the plugin with this:

packer.startup({
  function(use, use_rocks)
    use {
        "zbirenbaum/copilot.lua",
        event = {"VimEnter"},
        config = function()
            vim.defer_fn(function()
                require("copilot").setup()
            end, 100)
        end,
    }
    use {
        "zbirenbaum/copilot-cmp", -- should be used together with copilot.lua
        module = "copilot_cmp",
        config = function()
            require('copilot_cmp').setup()
        end,
    }
  end,
  ...
})

I don't see anything wrong with this config.

winkee01 commented 2 years ago

I made one more test and found that, If I choose the candidate with a full function body, it will not prompt the candidate list again, but If I choose the candidate with only a function name line, it will re-prompt me the candidate list:

https://user-images.githubusercontent.com/49930785/180125015-e2cc5e3d-7225-478c-a2de-9e3f2832efb3.mp4

yngwi commented 2 years ago

@winkee01 I think I may have similar behavior. I tried having insert it several code snippets (see screenshot below) and for the things inserted, only the line // for i in 0..10 { triggered the behavior (in the screenshot the prompt is still open even after inserting the line). To me it seems as if the prompt stays open if the code is something not complete, the function below for instance was inserted completed, and the print statement as well, both didn't have the staying-open prompt, however, the comment did.

image

zbirenbaum commented 2 years ago

I seem to have discovered the crux of the problem but am at a loss on how to fix it. The only way I can get the the duplicate to disappear is by not editing the text to be inserted, which causes indentation issues. If I format it properly, even if I format the filter text and label in the same way the issue returns. This is problematic, as obviously no one wants their completion items to have a ton of indentation in the entries, and no one wants to have to fix their indentation levels every time they insert code. Even if I managed to fix the indentation, the nature of the issue makes me almost certain this issue would persist given the modifications to the inserted text I do to account for lines or characters that already exist.

Unfortunately, nvim-cmp doesn't really have any in depth documentation on the more obscure parts of the source/lsp api regarding the method of text insertion and the optional fields such as filterText which might affect how entries are matched. I reviewed some old issues and discussions around the relevant topics and looked through the source code of both cmp and a number of other source providers, but it is extraordinarily difficult to debug and resources are limited.

All that is to say, a hacky temporary solution isn't possible here and more robust handling is required. Thank you to everyone who provides diagnostic info, as it helped me track down the real problem. I'll try to work on this a bit over the next couple days, but it is very plausible that I will have to PR some functionality upstream to solve it. If it comes to that, this may take a while longer to fix, as cmp simply wasn't designed with the kinds of wildly off-spec completions that copilot provides.

Apologies for the long read but I wanted to keep everyone in the loop given that I'm 99.9% certain almost every other erratic behavior with copilot completions that I've noticed comes from the same root problem as this, making it the most severe bug in copilot-cmp + copilot.lua by far

zbirenbaum commented 2 years ago

Ok, so after debugging the cmp matching system I have determined the issue is indeed upstream and will likely need to work with hrsh7th to fix this, however I was quite luckily wrong about a hacky solution not being possible. The best temporary solution I could come up with was to change the filter text to be one character shorter than the actual text inserted. This will not affect the text inserted by your completion in any way, only its appearance in the menu. The reason this is a temporary solution is that as a very notable side effect of this, if you have typed the same number of characters as a completion contains you will no longer see that completion if you have a typo character or something within it as I believe cmp normally allows for. This is the best short term solution I can do while I try to find a more robust way of fixing it upstream.

I just pushed the change, so please :PackerSync and see if it solves the problem for now, and please note any side effects that might occur on this thread.

winkee01 commented 2 years ago

Hi, just got a chance to test the new push, however, the problem still exists.

As shown, a candidate list re-appears when I choose a candidate that has only a function name instead of a body. It seems that it just keeps trying to find a more refined candidate list after the first confirmation.

https://user-images.githubusercontent.com/49930785/180469077-4bdb5226-2816-4a31-a812-955b2ab4d30b.mp4

winkee01 commented 2 years ago

in spite of the double prompt problem, it also falsely indents the completion code

https://user-images.githubusercontent.com/49930785/180470904-4c6d16e4-6dc0-470d-9ad1-51ee120c0985.mp4

zbirenbaum commented 2 years ago

in spite of the double prompt problem, it also falsely indents the completion code

https://user-images.githubusercontent.com/49930785/180470904-4c6d16e4-6dc0-470d-9ad1-51ee120c0985.mp4

Now that is really strange. I'll take a look at this

zbirenbaum commented 2 years ago

@winkee01 I pushed an update to copilot-cmp earlier that may fix it for you. Try it out and let me know if anything changed.

winkee01 commented 2 years ago

Just tested it, and nothing has changed, the problem is still there.

zbirenbaum commented 2 years ago

Just tested it, and nothing has changed, the problem is still there.

I pushed updates to both copilot-cmp and copilot.lua. There's now an autoformatter function that runs on the range lines inserted by completion (same formatting that would be done by visually selecting the inserted text and pressing = or gg=G which would do it to the whole file)

The autoformatter is enabled by default. I can't reproduce the issues you have in the video after adding it, so if you still have trouble I am going to need you to supply every indent related variable set such as shiftwidth indentexpr expandtab etc. Vim has a 'recommended' format setting for some langs like rust and python which overrides indent related vars (which I turn off) which might be why you are experiencing such strange behavior.

Also, I don't see your full cmp config here, so please post that as well if you continue to have issues. @winkee01

zbirenbaum commented 2 years ago

Just tested it, and nothing has changed, the problem is still there.

Does the problem still exist?

winkee01 commented 2 years ago

Unfortunately yes, it still exists.

-----------------------------------------------------------------------------//
-- Indent
-----------------------------------------------------------------------------//
vim.opt.wrap        = true
vim.opt.wrapmargin  = 2
vim.opt.textwidth   = 80
vim.opt.autoindent  = true
vim.opt.shiftround  = true
vim.opt.shiftwidth  = 4
vim.opt.tabstop     = 4
vim.opt.softtabstop = 4
vim.opt.expandtab   = true
vim.opt.hlsearch    = true
vim.opt.incsearch   = true

Did you make the exact same test? cause when I repeated the test every time, it automatically back indented.

dsully commented 2 years ago

I'm seeing similar with Python code, in that there is no indentation, the inserted text from Copilot is pushed against the left column.

Not sure if this is the same issue or something else.

zbirenbaum commented 2 years ago

I'm having a lot of trouble reproducing this. It seems to be working fine for me. I thought it may be an issue on the copilot end, so I tried updating the lsp agent a while ago, but the last couple copilot versions have major issues (not working and generate multi gigabyte log files and max out CPU usage) and the copilot discussion forum is filled with people reporting problems across platforms, so that's not really an option right now. Copilot

dsully commented 2 years ago

This is what I'm seeing (again, might be different issue than originally raised in this ticket):

https://user-images.githubusercontent.com/22371/187280247-efe4994d-2ca9-4480-89aa-997ba9a5d4a5.mp4

zbirenbaum commented 2 years ago

Is this still an issue? A ton of the logic has been cleaned up and I haven't run into it again.