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

[FEATURE] Allow manual completion via function #149

Closed
Guergeiro opened this issue Feb 12, 2024 · 9 comments · Fixed by #70
Closed

[FEATURE] Allow manual completion via function #149

Guergeiro opened this issue Feb 12, 2024 · 9 comments · Fixed by #70

Comments

@Guergeiro
Copy link

Is your feature request related to a problem? Please describe.
When using this plugin, it is recommended that we map the accept_key to something that doesn't collide with other plugin mappings. While this works, it would be interesting if we can do a manual complete ourselves by using some kind of provided function. Generally, autocomplete engines provide ways of doing this (asyncomple.vim, ddc.vim, ncm2, CoC, nvim-cmp etc).

Describe the solution you'd like
I'm going to type vimscript since I don't know how to do it in lua in neovim (migrating atm):

inoremap <silent> <expr> <cr>
    \ tabnine#has_suggestion() ?
    \ tabnine#confirm() : '<cr>'

Describe alternatives you've considered
The only alternative I've considered is having some function as accept_key that we can have logic attached to it that decides whether or not we want to return a valid keybind or just nil.

Additional context
Although this plugin doesn't serve as a autocomplete plugin, it very much has autocomplete functionality. Having an API that allows us to inspect the results of this plugin could even benefit other users of it; cmp-tabnine comes to mind.

@aarondill
Copy link
Contributor

i've had a pr waiting over 10 months over at #70 to implement this exact feature! i'd really appreciate your feedback on that pr if you have any

@Guergeiro
Copy link
Author

i've had a pr waiting over 10 months over at #70 to implement this exact feature! i'd really appreciate your feedback on that pr if you have any

Glancing over it with my limited lua knowledge, it seems that you went with the alternative suggestion described in my issue. It seems like it should work, although personally I'd rather have the functions exposed by tabnine-vim like described above (or equivalents).

Will probably use your fork if no further development is made on this issue.

@aarondill
Copy link
Contributor

the original goal of my pr was to implement it with as small of a diff as possible.

if I get a go ahead from @amirbilu, I would also prefer to have separate has_completion and complete functions

@aarondill
Copy link
Contributor

aarondill commented Mar 24, 2024

@Guergeiro amir has given his approval for me to go ahead and make these changes!
I plan to add this interface:

require("tabnine").accept_suggestion() -- return boolean: success (maybe?) else no return - NOTE: no error if no suggestion!
require("tabnine").has_suggestion() -- return boolean - has
require("tabnine").dismiss_suggestion() -- see above for accept_suggestion

do you have any feedback on this api? would this work for your use case?

@aarondill
Copy link
Contributor

secondly, do you think it makes more sense to expose these under require'tabnine' or require'tabnine.keymaps'? I haven't decided and amir doesn't seem to have an opinion either way. As a potential user of this API, i'd like to see what's most intuitive to you.

@amirbilu
Copy link
Contributor

We're already exposing require('tabnine.status').status() .. so I would go with tabnine.keymaps

@Guergeiro
Copy link
Author

The functions provided seem nice. Where should they be namespaced to, is a matter for the maintainers of the package, but from personal opinion, keymaps doesn't particularly sit well with me.

Since this is a neovim plugin, why don't you look into how nvim-cmp does this?

@aarondill
Copy link
Contributor

@Guergeiro nvim-cmp does this:

cmp.mapping = require('cmp.config.mapping')

which is essentially namespacing it into .mapping

I think i'll stick with tabnine.keymaps, since that's what amir expressed is his preference

@Guergeiro
Copy link
Author

I've just finished reading their doc.txt and they have it at top level with cmp.visible, cmp.complete, cmp.abort, cmp.close, etc.

I think you are mixing the functions that interact with tabnine with the keybinds that call these functions. The latter would make sense to be namespaced into something like keybinds.

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 a pull request may close this issue.

3 participants