devilspie2 icon indicating copy to clipboard operation
devilspie2 copied to clipboard

Support `require`ing modules located within the script folder

Open mark-cooke opened this issue 11 months ago • 0 comments

What was intended to be a fairly small PR, to include the script folder in Lua's package search path, ended up getting a little more involved:

Lua tries to only load a module once, no matter how many times it's required, but the default behaviour of assigning all unlisted files to an implicit scripts_window_open table renders that mechanism inoperable/ineffective.

This PR resolves this by:

  1. Adding support for a scripts_window_open variable.
  2. Allowing the scripts... variables to be assigned a single string, referencing a single file.
  3. Re-initialising the Lua VM when files/config have changed (needed for module changes to be "caught" since they're only loaded once) but imho it's also safer, anyway, if the changed files stored and referenced global data.

In support of the above changes I also:

  • Updated the README.md to reflect these changes and attempted to simplify and organise the information a little more clearly.
  • Refactored add_lua_file_to_list to accept the script folder path and include the call to g_build_path since the same thing was being done in a few places.
  • Pulled the 2 parts of the directory monitor handler that dealt with config reload out into a function (refresh_config_and_script).
  • Removed the unused file counter from load_config.
  • Followed clang's advice re const on args (of fuctions related to the changes).
  • Fixed a few memory leaks (found using Valgrind)

Why not other implementations?

  1. A variable/function to disable the "greedy loading"
    • You'd also need a scripts_window_open, anyway, and have 2 things to add to config when 1 would do the same job.
  2. An "exclude_files" list/table
    • Inconsistent with the rest of the script_... tables. Would also require more processing to end up with, basically, the same "window open" list anyway. Also begs a question of would it only apply to the "open" list, or all?

@dsalt This is one of a total of 5 PRs so I'm sorry for the github spam! If you decide to accept them and would rather I group them into 1 larger PR, just let me know.

Thanks!

mark-cooke avatar Jan 20 '25 20:01 mark-cooke