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

complete_info() returns incorrect index with behavior that depends on completeopt #12230

Closed
d-r-a-b opened this issue Apr 5, 2023 · 4 comments
Labels

Comments

@d-r-a-b
Copy link

d-r-a-b commented Apr 5, 2023

Steps to reproduce

complete_info() returns incorrect index programmatically, causing problems for completion plugins relying on complete_info() for native pumenu interoperability. There is some subtle change in behavior depending at least on if completopt includes noselect.

See hrsh7th/nvim-cmp#1326 for more context on how this bug was discovered and neovim/neovim#22892 for the initial bug report there.

Repro path

  • Save this file as minimal_vimrc
    set completeopt=menu,preview
    set cpoptions-=<
    nmap \\ iaaa<CR>bbb<CR>ccc<CR>ddd<CR>eee<CR>fff<CR>
    imap , <Cmd>put =complete_info(['selected'])['selected']<CR>
  • vim --clean -u minimal_vimrc
  • normal mode \\, which is nmapped to give minimal example file
    aaa
    bbb
    ccc
    ddd
    eee
    fff
    
    and will put you in insert mode on a newline
  • insert mode <C-X><C-L> to open lines pumenu -- with default completeopt=menu,preview will select "fff"
  • insert mode <C-P>, to navigate up the list once -- selecting "eee"
  • insert mode ,,,,, imapped to put into the buffer the value of complete_info index
    - The , was hit multiple times so that the complete_info index isn't hidden behind the pumenu and you can see the value of the index

BUG: The inserted index value is 1 when it should be 4

The bug behavior is that index counts from the bottom instead of from the top. However, whether it counts from top or bottom depends on completeopt value.

  • completeopt=menu,preview
    • behavior described above and seems to always count from bottom
  • if completeopt=menu,preview,noselect, then the indexing shows 2 divergent behaviors
    • navigate pumenu with <C-P> first --> you get the reversed index (counting from bottom instead of top), acting like the menu,preview case
    • navigate pumenu with <C-N> first --> you get the expected index, counting from top

Expected behaviour

Hitting , should insert 4 from it's call to vim.fn.complete_info({'selected'}).selected

Version of Vim

VIM - Vi IMproved 9.0 (2022 Jun 28, compiled Mar 21 2023 20:51:42) Included patches: 1-1420 Compiled by Arch Linux

Environment

Operating System: Arch LInux, on WSL2 on Win11
Terminal: windows terminal
$Term: xterm-246color
Shell: bash

Logs and stack traces

No response

@d-r-a-b d-r-a-b added the bug label Apr 5, 2023
@brammool
Copy link
Contributor

brammool commented Apr 16, 2023 via email

@d-r-a-b
Copy link
Author

d-r-a-b commented Apr 16, 2023

Thanks for getting back to me and looking into it!

I guess it seems like a bug to me because (complete_info()['items'])[complete_info()['selected']] gives the wrong completion. The following vimrc demonstrates this

set completeopt=menu,preview
set cpoptions-=<
nmap \\ iaaa<CR>bbb<CR>ccc<CR>ddd<CR>eee<CR>fff<CR>
imap , <Cmd>put =complete_info(['selected'])['selected']<CR>
imap = <Cmd>put =execute('echo((complete_info()[''items''])[complete_info()[''selected'']])')<CR>

And enter the same key sequence as in the initial reproduction, just substituting the new = to insert what complete_info seems to be telling me is the selected item (\\<C-X><C-L><C-P>=)

image

Needing to account for forwards/backwards selection certainly seemed surprising to me and I would suspect that it would surprise many other people trying to use complete_info. complete_info() doesn't seem to expose any information on whether the search is currently in forwards mode or backwards mode, so there's no way to tell if the selected index needs to be adjusted in order to actually use it to index into items. Even if it did, It also seems like a clunky interface to have to calculate the length of complete_info()['items'] and subtract the selected index from it depending on the direction, but I can understand that this might have been done for historical reasons/consistency of behavior with other interfaces. I can only say that coming from other projects/interfaces this behavior really surprises me.

Is this forwards/backwards information exposed elsewhere? Was there a part of the documentation that was supposed to help me know I needed to account for direction? Is there a better way to get the current completion item?

@d-r-a-b
Copy link
Author

d-r-a-b commented Apr 16, 2023

Note that for completion with CTRL-L the search goes backwards. Thus
"fff" is the first entry found. That this is called "1" actually makes
a lot of sense.

As stated in the original report, search is consistently backwards when set completeopt=menu,preview. However, it is inconsistent when completeopt=menu,preview,noselect. Then direction may be either forwards or backwards depending on whether <C-P> or <C-N> is hit first to navigate the menu.

@brammool
Copy link
Contributor

brammool commented Apr 17, 2023 via email

LemonBoy added a commit to LemonBoy/vim that referenced this issue Aug 30, 2023
Start the iteration from the same point and follow the same direction as
done when assigning the completion numbers. This way we remove the
dependance on the completion direction and make the order of 'info'
consistent.

Fixes vim#12230
zeertzjq added a commit to zeertzjq/neovim that referenced this issue Oct 11, 2023
Problem:  complete_info() returns wrong index
Solution: Make order of 'info' in completion_info consistent

Start the iteration from the same point and follow the same direction as
done when assigning the completion numbers. This way we remove the
dependence on the completion direction and make the order of 'info'
consistent.

closes: vim/vim#12230
closes: vim/vim#12971

vim/vim@69fb5af

Co-authored-by: LemonBoy <thatlemon@gmail.com>
zeertzjq added a commit to neovim/neovim that referenced this issue Oct 12, 2023
Problem:  complete_info() returns wrong index
Solution: Make order of 'info' in completion_info consistent

Start the iteration from the same point and follow the same direction as
done when assigning the completion numbers. This way we remove the
dependence on the completion direction and make the order of 'info'
consistent.

closes: vim/vim#12230
closes: vim/vim#12971

vim/vim@69fb5af

Co-authored-by: LemonBoy <thatlemon@gmail.com>
jaehoonhwang pushed a commit to jaehoonhwang/neovim that referenced this issue Oct 22, 2023
Problem:  complete_info() returns wrong index
Solution: Make order of 'info' in completion_info consistent

Start the iteration from the same point and follow the same direction as
done when assigning the completion numbers. This way we remove the
dependence on the completion direction and make the order of 'info'
consistent.

closes: vim/vim#12230
closes: vim/vim#12971

vim/vim@69fb5af

Co-authored-by: LemonBoy <thatlemon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants