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

feat(completion): add pum_matches key to complete_info() #28576

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

famiu
Copy link
Member

@famiu famiu commented Apr 30, 2024

Problem: Currently there is no way of getting the list of completion items that are actually put in the popupmenu. Which means completion providers have no way of knowing if a completion was exhausted.

Solution: Add a pum_matches key to complete_info() which contains the indices of items that are put in the popupmenu.

Ref: vim/vim#10007

@github-actions github-actions bot added the completion Nvim built-in (omni)completion label Apr 30, 2024
@famiu famiu force-pushed the feat/vvars/complete_items branch 2 times, most recently from 3216ece to 3bc1b7e Compare April 30, 2024 05:04
@glepnir
Copy link
Member

glepnir commented Apr 30, 2024

don't get point. When there are no candidate words, that is when match_array is empty, the completechanged event will still be triggered ? idk why there need a new vvar ..complete_info also provide some infomations..

@famiu
Copy link
Member Author

famiu commented Apr 30, 2024

don't get point. When there are no candidate words, that is when match_array is empty, the completechanged event will still be triggered ? idk why there need a new vvar ..complete_info also provide some infomations..

Yes but you still need to know if completion is exhausted or not, and what the completion items are. Currently there is no way of getting that. Also, look at vim/vim#10007 to know why complete_info() is not enough

@famiu famiu force-pushed the feat/vvars/complete_items branch from 3bc1b7e to b673df5 Compare April 30, 2024 06:03
@glepnir
Copy link
Member

glepnir commented Apr 30, 2024

It seems that because the items of complete_info are all items, it cannot distinguish the candidate words displayed in the current pum. I implemented similar behavior locally in my completion plugin. Now the order of items will not be disrupted #27955 so you can easily infer the candidate words displayed in the current pum. can't simply add the current index in pum to items, for example the shown_idx field? This should be easier than adding a new vvar.

@famiu famiu force-pushed the feat/vvars/complete_items branch from b673df5 to afdf54d Compare April 30, 2024 08:58
@famiu famiu changed the title feat(completion): add v:complete_items to get completion items feat(completion): add pum_items key to complete_info() Apr 30, 2024
@famiu famiu force-pushed the feat/vvars/complete_items branch 3 times, most recently from daa01d2 to 6a0ea9f Compare April 30, 2024 09:20
@famiu famiu marked this pull request as ready for review April 30, 2024 09:39
@famiu famiu force-pushed the feat/vvars/complete_items branch from 6a0ea9f to de202e2 Compare April 30, 2024 11:44
@famiu famiu changed the title feat(completion): add pum_items key to complete_info() feat(completion): add shown key to complete_info() Apr 30, 2024
@famiu famiu force-pushed the feat/vvars/complete_items branch from de202e2 to 40ec8dd Compare April 30, 2024 11:47
runtime/doc/builtin.txt Outdated Show resolved Hide resolved
@famiu famiu force-pushed the feat/vvars/complete_items branch from 40ec8dd to 0ebc8db Compare April 30, 2024 12:44
@famiu famiu changed the title feat(completion): add shown key to complete_info() feat(completion): add visible key to complete_info() Apr 30, 2024
@famiu famiu requested a review from justinmk April 30, 2024 13:11
@zeertzjq
Copy link
Member

The name "visible" may also be ambiguous. If there are many matches, there will be a scrollbar and some matches are only shown after scrolling. Should those be considered "visible" or not?

@zeertzjq zeertzjq added this to the 0.11 milestone Apr 30, 2024
@justinmk
Copy link
Member

If there are many matches, there will be a scrollbar and some matches are only shown after scrolling. Should those be considered "visible" or not?

And that should be spelled out in the docs. I thought for sure that the purpose of this PR was to indicate which items are visible on the screen. What, then, is the purpose? (Say it in the docs, not to me)

@echasnovski
Copy link
Member

The name "visible" may also be ambiguous. If there are many matches, there will be a scrollbar and some matches are only shown after scrolling. Should those be considered "visible" or not?

True. Maybe "matched" then?

@famiu
Copy link
Member Author

famiu commented Apr 30, 2024

The name "visible" may also be ambiguous. If there are many matches, there will be a scrollbar and some matches are only shown after scrolling. Should those be considered "visible" or not?

True. Maybe "matched" then?

This doesn't work because completion items are called completion matches in several places in the documentation.

I feel like at the end of the day, pum_items probably makes the most amount of sense, or pum_indices or something that makes it clear that we're referring to popupmenu items. Maybe pum_matches?

Problem: Currently there is no way of getting the list of completion items that are actually put in the popupmenu. Which means completion providers have no way of knowing if a completion was exhausted.

Solution: Add a `pum_matches` key to `complete_info()` which contains the indices of items that are put in the popupmenu.

Ref: vim/vim#10007
@famiu famiu force-pushed the feat/vvars/complete_items branch from 0ebc8db to c4f0235 Compare April 30, 2024 14:59
@famiu famiu changed the title feat(completion): add visible key to complete_info() feat(completion): add pum_matches key to complete_info() Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completion Nvim built-in (omni)completion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants