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

Plugin package path #4248

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

oscarcederberg
Copy link
Contributor

@oscarcederberg oscarcederberg commented Sep 4, 2023

Related to #4150

Allows for wezterm plugins to require separate scripts besides plugin/init.lua in the plugin's directory.

An example plugin is available: oscarcederberg/plugin-package-path.wezterm.
With this change, including the plugin will run the following scripts:
<PLUGIN_DIR>/plugin/init.lua -> <PLUGIN_DIR>/other_module/init.lua -> <PLUGIN_DIR>/other_module/other_script.lua.

Adds <DATA_DIR>/?/.init.lua and <DATA_DIR>/?.lua as searchable package paths. The package path is now in this order on a laptop running Linux (logged by calling wezterm.log_info(package.path), formatted for readability):

<HOME_DIR>/.config/wezterm/?.lua;
<HOME_DIR>/.config/wezterm/?/init.lua;
<HOME_DIR>/.local/share/wezterm/plugins/?/plugin/init.lua;
<HOME_DIR>/.local/share/wezterm/plugins/?.lua;             <-- Inserted
<HOME_DIR>/.local/share/wezterm/plugins/?/init.lua;        <-- Inserted
<HOME_DIR>/.wezterm/?.lua;
<HOME_DIR>/.wezterm/?/init.lua;
/usr/local/share/lua/5.4/?.lua;
/usr/local/share/lua/5.4/?/init.lua;
/usr/local/lib/lua/5.4/?.lua;
/usr/local/lib/lua/5.4/?/init.lua;
./?.lua;./?/init.lua

I didn't find myself anywhere appropriate to document this change in the code. I also tried running all tests, but my laptop doesn't seem to be able to handle them and freezes unfortunately.

Kind regards! 😃

@wez
Copy link
Owner

wez commented Sep 4, 2023

I'm a bit confused by this:

https://github.com/oscarcederberg/plugin-package-path.wezterm/blob/e3ebe66573d31bec22db641eaaeb3dd9f58aafc5/plugin/init.lua#L6

My expectation is that that will try to pull in plugin/other_module.lua but from the layout of your example repo it looks like it is effectively pulling in plugin/../other_module/init.lua, which feels surprising in a potentially bad way.

My expectation is based on thinking that all of the lua code in the plugin repo would belong under its plugin directory, and not be outside of that location.

It seems to me that my choice of ?/plugin/init.lua is problematic.

What if we redefined (or rather, extended, in order to be backwards compatible with the couple of existing plugins) the plugin layout to be plugin/<plugin-name>/init.lua?
Then the existing plugin include path added by config/src/lua.rs could be changed to:

  • {}/plugins/?/plugin/?/init.lua
  • {}/plugins/?/plugin/?.lua

From my reading of https://www.lua.org/manual/5.3/manual.html#pdf-package.searchpath I think it is valid to list ? multiple times in the same path. I'm not sure if that solves things though.

Another other option for this stuff is not to use require for loading submodules. You could potentially do something like this, which just loads files by path:

local wezterm = require("wezterm")
local module = {}
local path = ...

-- Equivalent to POSIX dirname(3)
-- Given "/foo/bar" returns "/foo/"
-- Given "c:\\foo\\bar" returns "c:\\foo\\"
function dirname(s)
  return string.gsub(s, '(.*[/\\])(.*)', '%1')
end

local other_module = dofile(dirname(path) .. "other_module.lua")

return module

I think it's reasonable to add dirname and basename functions to the wezterm module to save everyone from defining a version of it.

@oscarcederberg
Copy link
Contributor Author

Hi!

My expectation is that that will try to pull in plugin/other_module.lua but from the layout of your example repo it looks like it is effectively pulling in plugin/../other_module/init.lua, which feels surprising in a potentially bad way.

My expectation is based on thinking that all of the lua code in the plugin repo would belong under its plugin directory, and not be outside of that location.

I think you're right.

plugin layout to be plugin/<plugin-name>/init.lua? Then the existing plugin include path added by config/src/lua.rs could be changed to:

* `{}/plugins/?/plugin/?/init.lua`

* `{}/plugins/?/plugin/?.lua`

I've tried to look it up, but I don't think it is possible to "break-up" a module-name. If I understand you correctly, you'd want it to go from <PLUGIN_ENCODED_NAME>.<MODULE>.<SCRIPT> to e.g. {}/plugins/<PLUGIN_ENCODED_NAME>/plugin/<MODULE>/<SCRIPT>.lua? I don't know if that is possible.

If ? is included several times in a path, it seems to expand each to the full searched script-name. The above becomes {}/plugins/<PLUGIN_ENCODED_NAME>/<MODULE>/<SCRIPT>/plugin/<PLUGIN_ENCODED_NAME>/<MODULE>/<SCRIPT>.lua when I tried with {}/plugins/?/plugin/?.lua in package.path.

Another other option for this stuff is not to use require for loading submodules. You could potentially do something like this, which just loads files by path:

local wezterm = require("wezterm")
local module = {}
local path = ...

-- Equivalent to POSIX dirname(3)
-- Given "/foo/bar" returns "/foo/"
-- Given "c:\\foo\\bar" returns "c:\\foo\\"
function dirname(s)
  return string.gsub(s, '(.*[/\\])(.*)', '%1')
end

local other_module = dofile(dirname(path) .. "other_module.lua")

return module

I think it's reasonable to add dirname and basename functions to the wezterm module to save everyone from defining a version of it.

Yes, this is also an alternative probably in case we don't find a solution with require!

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