bug: hard to debug error when returning multiple return values from `import`ed module

[titaniumtraveler]

Did you check docs and existing issues?

• [X] I have read all the lazy.nvim docs
• [X] I have updated the plugin to the latest version before submitting this issue
• [X] I have searched the existing issues of lazy.nvim
• [X] I have searched the existing issues of plugins related to this issue

Neovim version (nvim -v)

NVIM v0.10.0

Operating system/version

arch linux

Describe the bug

When adding additional return values to a lua module that is imported from an import <module-directory> that seems to play badly with the way errors are reported from lazy.nvim about modules that failed to load correctly.

Steps To Reproduce

In lazy config:

{
  ...
  spec = { import = "plugin" },
  ...
}

In lua/plugin.lua:

return {}, {}

Error:

Error detected while processing ~/.config/init.lua:
Failed to load `plugin`

...local/share/nvim/lazy/lazy.nvim/lua/lazy/core/plugin.lua:184: attempt to concatenate local 'err' (a table value)

# stacktrace:
  - .config/nvim/lua/plugin.lua:1

I didn't manage to create a repro for it. Mostly because I don't know how I would demonstrate import in it, but I hope this is okay in this form.

Note if instead of the empty table, a string is chosen as second return parameter, there is no stacktrace and it just returns the string as the error:

return {}, "test string"

Error detected while processing ~/.config/nvim/init.lua:
Failed to load `plugin`:
test string

Expected Behavior

IMO this should either:

1. Return an explicit error about not supporting multiple return values.
2. Just support multiple return values and merge them as regular plugin specs.
3. Ignore any following return values and output a warning about it.
4. Or silently ignore the following return values.

I prefer option 3, but 1 or 2 would be fine too and 4 feels like it would lead to accidental misuse of lazy.nvim.

Repro

vim.env.LAZY_STDPATH = ".repro"
load(vim.fn.system("curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua"))()

require("lazy.minit").repro({
  spec = {
    -- add any other plugins here
  },
})

[max397574]

I think that's because in the code there is (just assuming, haven't checked) a local ok, err = pcall(require, "plugins") Normally ok would be the return value and err the error. So not much there can be done about this

Also the docs are imo quite clear about this. It says you should return a table of plugins which you aren't doing here.

IMO this should either:

1. Return an explicit error about not supporting multiple return values.

complicated to do with the current setup. Would require an additional check which could perhaps also impact performance. People should just read the docs and don't do it wrong. Otherwise this is something quite easy to fix if people open issues about it (I haven't seen many (if any) of those kind here though)

2. Just support multiple return values and merge them as regular plugin specs.

Kind complicated in the code to work with an unknown amount of return values.

3. Ignore any following return values and output a warning about it.

Would perhaps be an acceptable approach

4. Or silently ignore the following return values.

That would just be really confusing for users. People would open issues because "lazy doesn't work" when adding new plugins

[titaniumtraveler]

I think that's because in the code there is (just assuming, haven't checked) a local ok, err = pcall(require, "plugins") Normally ok would be the return value and err the error. So not much there can be done about this

Ah, not exactly, but kind of. The actual error originates in this code: https://github.com/folke/lazy.nvim/blob/077102c5bfc578693f12377846d427f49bc50076/lua/lazy/core/plugin.lua#L181-L195 As I understand it, when modspec.load() returns multiple values in mod, that basically "pushes" err out and replaces that with the returned type. That's also the reason that you can just straight up replace parts of the error message when you replace the {} with some string. (I haven't really looked deeper than that, because behind modspec.load() are a few layers of indirection, which I didn't really want to search through at that moment.)

Also the docs are imo quite clear about this. It says you should return a table of plugins which you aren't doing here.

I mean yes the docs are clear about this. But my point is that lazy.nvim should still not implicitly error because of this. And return {"a", "b"} and return "a", "b" are too similar that beginners might accidentally forget the {}, or someone could just make a typo.

IMO this should either:

1. Return an explicit error about not supporting multiple return values.

complicated to do with the current setup. Would require an additional check which could perhaps also impact performance. People should just read the docs and don't do it wrong. Otherwise this is something quite easy to fix if people open issues about it (I haven't seen many (if any) of those kind here though)

Not necessarily. In the best case it could be either solved by simply swapping mod and err, or by not returning the values directly in the implementation of modspec.load().

2. Just support multiple return values and merge them as regular plugin specs.

Kind complicated in the code to work with an unknown amount of return values.

In the best case it would just capture the values in a table and then iterate over them. (Something like local mods = { modspec.load() }. Not exactly that, but you get the gist.)

3. Ignore any following return values and output a warning about it.

Would perhaps be an acceptable approach

4. Or silently ignore the following return values.

That would just be really confusing for users. People would open issues because "lazy doesn't work" when adding new plugins

Yeah, like I wrote it wouldn't be my favorite solution, but for completeness sake I wanted to have it mentioned.

[folke]

Just return what is expected.