Skip to content

Added a preselect option to the cmp menu #24

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

Merged
merged 10 commits into from
Oct 9, 2024
Merged

Added a preselect option to the cmp menu #24

merged 10 commits into from
Oct 9, 2024

Conversation

BrianNormant
Copy link
Contributor

No description provided.

@BrianNormant
Copy link
Contributor Author

To fix #14.

Copy link
Owner

@Saghen Saghen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I'd prefer a boolean has_selected that gets reset to config.windows.autocomplete.preselect on new context.id values. Would be great to have a set_has_selected that handles showing/hiding cursorline as well, making sure to double check autocomplete.win:is_open()

@BrianNormant
Copy link
Contributor Author

BrianNormant commented Oct 9, 2024

Something I've noticed when testing: if we continue typing while the autocompletion window is opened, it stays open and the result just get filtered. My question is, when in "not preselect" mode, should the selection reset when the completions items get updated?

In not preselect mode, If the user native through the items but decide to choose none, he doesn't expect the selection to jump back to first item as he continue typing. He would simply use Tab and S-Tab to select an item if he want to accept this new suggestion. Thus I think it's better to reset the selection ( has_selected = false ) when the list get updated.

@Saghen
Copy link
Owner

Saghen commented Oct 9, 2024

Yep, good point. I believe you'd want to check if the cursor position changes, rather than checking if the items have changed, since items can be updated twice per keystroke. On keystroke, the list gets immediately re-filtered with stale data. The sources are called and the list gets updated again asynchronously, if there's new data

@BrianNormant
Copy link
Contributor Author

Hmm, I didn't run into this, so it'd be hard to check. As far as I know this shouldn't impact this PR much? the only modification are in the select_prev/next function.

function autocomplete.select_prev()
  if not autocomplete.win:is_open() then return end

  local cycle_from_top = config.windows.autocomplete.cycle.from_top
  local l = #autocomplete.items
  local line = vim.api.nvim_win_get_cursor(autocomplete.win:get_win())[1]
  if line <= 1 then
    if not cycle_from_top then return end
    line = l
  else
    line = line - 1
  end

  autocomplete.set_has_selected(true)

  autocomplete.win:set_option_values('cursorline', true)
  vim.api.nvim_win_set_cursor(autocomplete.win:get_win(), { line, 0 })
  autocomplete.event_targets.on_select(autocomplete.get_selected_item(), autocomplete.context)
end

Which get line info from the current cursor position.
What could happen if I understand what you are saying is that autocomplete.open_with_items(context, items) gets called with new items, resetting the cursor position because of this line: autocomplete.set_has_selected(autocmp_config.preselect).
But I'm not sure what to do if this is the case. the items are changed, and the item the user was on could be removed. So where should the selection/cursor go?

Comment on lines +157 to +160
cycle = {
from_bottom = true,
from_top = true,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These configs are already documented as part of windows.autocomplete:

cycle = {

Suggested change
cycle = {
from_bottom = true,
from_top = true,
},

from_bottom = true,
from_top = true,
},
preselect = true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant to put this in the windows.autocomplete section, as per:

preselect = true,

@BrianNormant
Copy link
Contributor Author

You're right, I forgot to stage those, my bad

@Saghen
Copy link
Owner

Saghen commented Oct 9, 2024

Thanks again, I'll wrap up the remaining small changes on main

@Saghen Saghen merged commit 1749e32 into Saghen:main Oct 9, 2024
lopi-py pushed a commit to lopi-py/blink.cmp that referenced this pull request Oct 10, 2024
* feat: Added a preselect option to the cmp menu

* fix: implement requested changes

* fix: silly not

* fix: Completion select on the second line

Squash me!

* Add default config to the readme

* Fix: duplicated field in default config
lopi-py pushed a commit to lopi-py/blink.cmp that referenced this pull request Oct 10, 2024
* feat: Added a preselect option to the cmp menu

* fix: implement requested changes

* fix: silly not

* fix: Completion select on the second line

Squash me!

* Add default config to the readme

* Fix: duplicated field in default config
@sQVe
Copy link
Contributor

sQVe commented Oct 11, 2024

I'm a bit confused here. Is this feature wrapped up? I cannot seem to get it to work.

@BrianNormant
Copy link
Contributor Author

I´d assume you have one of the lastest commit and windows.autocomplete.preselect = false.
The only change you should see is no item is selected by default when the completion menu open.
image

When pressing one of the select_next or select_prev key, the first/last item should be selected, Indicated by a cursorline showing up. llike this:
image

If no item are selectec, pressing the accept key will simply execute it's normal action. In my case will simply insert a new line.
image

If a item was selected, It will insert it.
image

@sQVe
Copy link
Contributor

sQVe commented Oct 12, 2024

I'm on the latest version and, weirdly, that doesn't work.

@noomly
Copy link
Contributor

noomly commented Oct 12, 2024

If you pinned the plugin to 0.2.1 then that makes sense, the changes haven't been released yet

@sQVe
Copy link
Contributor

sQVe commented Oct 12, 2024

@noomly Yeah, that was my mistake. Silly. Got it working now! 👍🏼

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.

4 participants