tzachar / compe-tabnine

TabNine source for hrsh7th/nvim-compe
BSD 3-Clause "New" or "Revised" License
52 stars 3 forks source link

Only use vim.g.compe.source.tabnine keys if vim.g.compe exists #4

Closed BurnerWah closed 3 years ago

BurnerWah commented 3 years ago

Trying to read vim.g.compe.source.tabnine.max_line and vim.g.compe.source.tabnine.max_num_results when vim.g.compe doesn't exist (ex. because compe was configured in Lua) breaks all compe completions.

This should fix that.

BurnerWah commented 3 years ago

This could also be done with either of the following:

max_lines = (((vim.g.compe or {}).source or {}).tabnine or {}).max_line or 1000;
max_num_results = (((vim.g.compe or {}).source or {}).tabnine or {}).max_num_results or 20;

or

max_line = (vim.g.compe or {source = {tabnine = {}}}).source.tabnine.max_line or 1000;
max_num_results = (vim.g.compe or {source = {tabnine = {}}}).source.tabnine.max_num_results or 20;

But I'm not particularly fond of either syntax.

In practice, I don't think it's very likely for this source to run into a situation where vim.g.compe exists and vim.g.compe.source.tabnine doesn't.

tzachar commented 3 years ago

Thanks for the fix. The change you propose does solve the issue, but what if the user configs tabnine defaults through lua? I think the following is better:

max_lines = (vim.g.compe and vim.g.compe.source.tabnine.max_line) or compe.source.tabnine.max_line or 1000;
max_num_results = (vim.g.compe and vim.g.compe.source.tabnine.max_num_results) or compe.source.tabnine.max_num_results 20;

But this is also ugly. There should be a better way of configuring defaults in nvim lua plugins, Im just not that fluent in lua. What is the canonical way of handling this? Or, what should be the compe way?

BurnerWah commented 3 years ago

I tested it out a little, and it seems like the entire configuration is exposed through require('compe.config').get(). I'm not as sure whether it's actually supposed to be used for this, though, since it's not used by any of the built-in sources.

I can update the PR with that if you want me to though.

EDIT - also, compe.source doesn't exist at all so that doesn't work.

tzachar commented 3 years ago

Well, I think maybe we should ask around in hrsh7th/nvim-compe. Want to open an issue there and ask for guidence? @hrsh7th probably has a solution

BurnerWah commented 3 years ago

Looks like compe will have something to handle this in v3.

Until that's finished, this should work fine as a hotfix, or I can just hack something together locally.

tzachar commented 3 years ago

Ok, i'll merge it for now, and once compe has something more permanent we will use that.