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

Refactor(keymaps): export keymaps #70

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

aarondill
Copy link
Contributor

@aarondill aarondill commented Apr 30, 2023

This PR implements accept_suggestion, dismiss_suggestion, and has_suggestion to be used from user mappings.

It also permits setting accept_keymap and dismiss_keymap to false to avoid setting it (nil defaults to the default settings)

fixes #149

I've included an example use case under a new header in the README.

copied from the readme:

--- Example integration with Tabnine and LuaSnip; falling back to inserting tab if neither has a completion
vim.keymap.set("i", "<tab>", function()
  if require("tabnine.keymaps").has_suggestion() then
    return require("tabnine.keymaps").accept_suggestion()
  elseif require("luasnip").jumpable(1) then
    return require("luasnip").jump(1)
  else
    return "<tab>"
  end
end, { expr = true })

@amirbilu
Copy link
Contributor

amirbilu commented May 3, 2023

Hi @aarondill
Sorry about the delay. Super busy week. Will try to get it by next week.

Did you get the swag already 😎?

@aarondill
Copy link
Contributor Author

aarondill commented May 3, 2023

Hi @aarondill Sorry about the delay. Super busy week. Will try to get it by next week.

No problem, take your time.

Did you get the swag already sunglasses?

I did! I sent a few images to your email address already, did you not receive them?

@amirbilu
Copy link
Contributor

amirbilu commented May 3, 2023

I haven't! to amirbilu@tabnine or to support@tabnine? If you have a twitter handle - let us know so we can share your images & story at https://twitter.com/tabnine

@aarondill
Copy link
Contributor Author

@amirbilu any word on this PR? I'm still thinking perhaps exporting these under tabnine rather than tabnine.keymaps, but i'd like your input on that, as well as their final names.

@aarondill
Copy link
Contributor Author

@amirbilu what are your thoughts on this patch? I can say I've been happily using this in my own config for a month with no issues (see here), and would love to get it merged into the main repo. I'd still like your input on the module name, whether tabnine.keymaps.accept_completion() or else tabnine.accept_completion(), or perhaps even just tabnine.accept(), and same for dismiss.

@aarondill
Copy link
Contributor Author

@amirbilu please let me know how you would like this change to continue. This is vital to my personal use of the project, and I'd like to finalize the PR so it can be merged.

@aarondill
Copy link
Contributor Author

@amirbilu have you had a chance to take a look at this yet?

@aarondill
Copy link
Contributor Author

@amirbilu sorry if it feels like I'm pestering you about this PR, but this is an extremely important use case to me, and as you seem to be the sole owner/maintainer of this repo, I don't know who else to ping to get this reviewed and merged.

I am currently maintaining a fork to implement this behavior for my own use and I'd love to be able to resume using upstream if this could get merged.

My stated use case is to remap the tab key (usually used for completion) to a function callback that completes Tabnine if available, otherwise uses cmp (my lsp completion client). This allows seemless integration of the two, and avoids my current problem of the two fighting over my tab key (and Tabnine winning, regardless of completion availability, removing my ability to use cmp).

@aarondill
Copy link
Contributor Author

@amirbilu here's a friendly ping on this PR. It's now been waiting over 6 months for a review and I'd appreciate if you could take a quick look at it.

@aarondill
Copy link
Contributor Author

@amirbilu any update on this PR?

@amirbilu
Copy link
Contributor

@aarondill to be honest it feels like a big change and I can't find the time to think of the consequences

@aarondill
Copy link
Contributor Author

@amirbilu this is a fairly advanced use case, but i've been using it in my own configuration without issue for the last 9 months (I'm maintaining a fork with all my prs)

@aarondill
Copy link
Contributor Author

@amirbilu this pr would also solve #149 with slight modification (which i'm happy to make)

if I make these modifications, would you be interested in merging this PR now?

@amirbilu
Copy link
Contributor

@amirbilu this pr would also solve #149 with slight modification (which i'm happy to make)

if I make these modifications, would you be interested in merging this PR now?

willing to add this if it does not break anything

@aarondill
Copy link
Contributor Author

@amirbilu I've rewritten this PR with has_suggestion in mind.
Please review the new commits when possible 😄

@amirbilu
Copy link
Contributor

Awesome contribution

@amirbilu amirbilu merged commit 463b3e4 into codota:master Mar 27, 2024
1 of 2 checks passed
@aarondill aarondill deleted the refactor_export_keymaps branch March 27, 2024 14:09
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.

[FEATURE] Allow manual completion via function
2 participants