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

Return correct has_satellite #361

Merged
merged 2 commits into from
Nov 25, 2024
Merged

Return correct has_satellite #361

merged 2 commits into from
Nov 25, 2024

Conversation

adlerd
Copy link
Contributor

@adlerd adlerd commented Nov 4, 2024

Progress bars are being disabled even when satellite is not present. Returning false from satellite.lua in this case restores the progress bars.

Progress bars are being disabled even when satellite is not present. Returning false from satellite.lua in this case restores the progress bars.
@Julian
Copy link
Owner

Julian commented Nov 25, 2024

@adlerd sincere apoogies, somehow I totally missed this PR. I definitely do see something isn't working in this case but I honestly don't really understand why? Maybe you looked?

Specifically, we check against nil, and we seem to return nil... does Lua do something magical if you return nil from a module, or yeah I don't see why it's broken at the minute, which I'd like to understand before merging. Let me know if you've already had a look, and sorry again for only just noticing.

@adlerd
Copy link
Contributor Author

adlerd commented Nov 25, 2024

After a recent update I lost progress bars. I can reproduce locally that when I apply this patch, I get progress bars back; and when I undo this patch, I lose them again.

I can't explain it either in terms of the Lua book and I don't have any "real-world" Lua experience. However, without this patch, the following code does not call notify(), when run via :lua ex command. (With this patch, it does call notify() as expected.)

local has_satellite = require 'lean.satellite'
if not has_satellite then
  vim.notify("not has_satellite")
end

This behavior differs from my local installation of luajit when it requires a file containing just return, so there's either a behavior change due to lua version or something funny about neovim's require.

@adlerd
Copy link
Contributor Author

adlerd commented Nov 25, 2024

Ah. vim.print(tostring(require 'lean.satellite')) prints true, and :help require states,

If the loader
returns no value and has not assigned any value to
package.loaded[modname], then require assigns true to this
entry.

In theory, since Lua has multiple-returns, return might be returning no values, rather than nil; and in theory, the "loader" (which :help is a bit vague on) might be correctly passing through multiple values. Actually it doesn't seem to matter, since return nil also gives no progress bars and stores true in the table. neovim's require appears to "helpfully" reinterpret a nil return from modules as generic success, whereupon it stores true.

@Julian
Copy link
Owner

Julian commented Nov 25, 2024

Fun... OK. Thanks for tracking this down and for the PR!

@Julian Julian merged commit 1e266a2 into Julian:main Nov 25, 2024
6 checks passed
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