GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — refactor!: download packages asynchronously by diego-velez · Pull Request #487 · nvim-java/nvim-java

Conversation

@diego-velez

@diego-velez diego-velez commented Apr 6, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Problem

Downloading packages is synchronous and blocks the UI. Users cannot type or do anything in Neovim while packages are downloading. Furthermore, because the UI is blocked, users do not know that a package is being downloaded even though nvim-java is supposed to notify the user.

Solution

Download packages asynchronously.

Implementation

At the core, it seems to me that the issue was using the old vim.fn.system to execute programs. This PR swaps the old vim.fn.system for the newer vim.system API, and makes all executions asynchronous.

To make this work, I had to list out all the packages that needed to be installed, instead of installing each package one by one in setup. Then, I pass to the package manager the list of packages that need to be installed. The package manager asynchronously installs each package. When all packages are finished installing, the setup callback is called, and setup can finish.

The main breaking change is that users cannot call vim.lsp.enable('jdtls') after they call require('java').setup() because it no longer blocks, so nvim-java might not be setup before the setup function completes. Because of this, we would need to enable jdtls ourselves after nvim-java is setup. Otherwise, we could create a user autocommand, and have users setup their java tooling in that autocommand.

Caution

This PR has not been tested in Windows.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

logrusx, khaneliman, paulopatto, and hackardoX reacted with thumbs up emoji paulopatto and joaosimsic reacted with heart emoji paulopatto reacted with rocket emoji
All reactions
  • 👍 4 reactions
  • ❤️ 2 reactions
  • 🚀 1 reaction
@diego-velez
refactor!: download packages asynchronously
4a2f72e
khaneliman added a commit to khaneliman/khanelivim that referenced this pull request Apr 18, 2026
@khaneliman
feat(overlays): add nvim-java async download patch
fc6509d
nvim-java/nvim-java#487 adds support for async
downloading to prevent the UI locking.
s1n7ax
s1n7ax reviewed Jun 7, 2026
View reviewed changes

@s1n7ax s1n7ax left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Great motivation — the blocking setup is a real UX problem. A few correctness issues to address before this is mergeable:

lua/pkgm/downloaders/wget.lua — missing return after error
In the failure branch of vim.system's callback, on_finished(nil, err) is called but there's no return, so execution falls through and on_finished(self.dest, nil) is called too. Caller will be invoked twice with conflicting results. Compare with curl.lua which returns correctly.

Errors thrown from libuv callback thread — manager.lua download_package, powershell.lua
vim.system's callback runs on the libuv thread, not the main loop. err_util.throw / notify.error / any vim.api.* call from there is unsafe and can crash. Wrap the failure branches in vim.schedule(function() … end). extract_package correctly uses vim.schedule; the download error paths need the same.

install_all doesn't handle failures
If one install throws (via err_util.throw) inside a callback, the remaining counter never decrements and the final callback() never runs — setup hangs silently. Consider passing an error to on_finished and propagating it to install_all's caller.

Behavior change — vim.lsp.enable('jdtls') now called by plugin
This is a real breaking change for users who already enable jdtls themselves; they'll get double-enable. The PR description flags the breaking change but the README/docs aren't updated. Please document the new contract (either "don't call vim.lsp.enable('jdtls') yourself" or "we no-op if already enabled") and update README.

Type inconsistency — powershell.lua on_finished
Annotated as fun(file_path: string) with no nil/error, while curl.lua/wget.lua use fun(file_path: string|nil, err: string|nil). The dispatcher in manager.lua download_package expects the latter signature. Unify them.

Notify call from callback — manager.lua install
notify.info('Package installed successfully ' .. name) is in the download callback. Same scheduling concern as above.

Untested on Windows is noted in the PR — given the powershell path is the one with the trickiest scheduling, that's the riskiest spot.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
s1n7ax
s1n7ax reviewed Jun 7, 2026
View reviewed changes

@s1n7ax s1n7ax left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Good direction — blocking setup is a real UX problem. A few correctness issues inline.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread lua/pkgm/downloaders/wget.lua Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lua/pkgm/manager.lua Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lua/pkgm/manager.lua Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lua/java.lua Resolved

Uh oh!

There was an error while loading. Please reload this page.

diego-velez added 5 commits June 8, 2026 10:24
@diego-velez
fix: missing return in failure branch
1828fd5
@diego-velez
chore: add Nix flake dev shell files to gitignore
14daca7
@diego-velez
docs: update readme and vim documentation
ada870e
panvimdoc was used to auto-generate the vim doc file
@diego-velez
fix: wrap throw with vim.schedule
def3865
@diego-velez
refactor: propagate errors to install_all
735b5a9
@diego-velez
diego-velez requested a review from s1n7ax June 8, 2026 15:50
@diego-velez

diego-velez commented Jun 11, 2026

Copy link
Copy Markdown
Author

Great motivation — the blocking setup is a real UX problem. A few correctness issues to address before this is mergeable:

@s1n7ax issues fixed, let me know what you think

s1n7ax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@s1n7ax s1n7ax Awaiting requested review from s1n7ax

At least 0 approving reviews are required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@diego-velez @s1n7ax
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.