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

add build lua #133

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

add build lua #133

wants to merge 11 commits into from

Conversation

aarondill
Copy link
Contributor

feat: add a build.lua file

This is supported by lazy.nvim to automagically build the project.

Copy link
Contributor

@amirbilu amirbilu left a comment

Choose a reason for hiding this comment

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

Interesting! Can this be called in every initialization? Instead of being a build script? We previously discussed about this

@aarondill
Copy link
Contributor Author

I think we shouldn't call it on every initialization (because this makes network requests), but maybe if there's no binaries installed, then run it?

@amirbilu
Copy link
Contributor

amirbilu commented Nov 6, 2023

Yes, that's what I meant :).. this is how it works with other plugins

@aarondill
Copy link
Contributor Author

@amirbilu Having tried to implement this for a while, there doesn't seem to be an easy way without up-ending the rest of the codebase.

In order to build asynchronously (and not block the entire editor), I would have to change every invocation to TabnineBinary:start and :request to be callback-based.

I can do this if you'd like, but it would be a much larger PR and likely would be a (set of) breaking change(s).

I can build it synchronously if you'd like, but this would freeze the entire editor window.

@amirbilu
Copy link
Contributor

amirbilu commented Nov 6, 2023

Got you. What are you suggesting? What option is better in your opinion

@aarondill
Copy link
Contributor Author

Got you. What are you suggesting? What option is better in your opinion

Well, ideally, we’d go to a callback-based system, but I would rather not implement those changes, so perhaps we run it synchronously and just deal with the freeze? It should only happen once anyways, right?

@amirbilu
Copy link
Contributor

amirbilu commented Nov 8, 2023

Freezing the IDE sounds scary to me.. but I would not want to give up on this. Maybe I could find time to refactor this to async

This is supported by lazy.nvim to automagically build the project.
We don't care about stdout since it's just progress indicators. These are too noisy to notify on.
`/build.lua` is now just a light wrapper for tabnine.build -- to continue lazy.nvim support
This will only run once, and it checks to ensure the machine is supported before wasting resources/time.

If it fails, we warn and direct the user to create an issue if they can't run the script manually.
This is currently unused
This plays nicer with nvim-notify (A very common vim.notify replacement plugin), by showing only one message at a time, instead of stacking them.

When using stock nvim, this should make no difference.
This is now okay, due to the previous commit (a25ecf3), so these messages will now not stack when using vim-notify.

these show only one message at a time. For another example of a plugin which shows progress messages like this, see nvim-treesitter/nvim-treesitter.
@aarondill
Copy link
Contributor Author

I've rebased this to the current state of master (eb914c2)

@aarondill
Copy link
Contributor Author

@amirbilu Ive rethought this pr, and I think i can make it work asynchronously (I'm going to copy from nvim-treesitter's async install).

should I also add a :TabnineBuild and/or :TabnineChatBuild command here, or add that to a new PR after I finish work here?

@aarondill aarondill marked this pull request as draft May 20, 2024 23:04
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.

2 participants