require-macros hook

[rktjmp]

Hotpot tracks "macro files" as dependencies for regular Fennel code, so if a macro is edited, any files that import-macro'd that file are also re-compiled.

It does this by a custom macro searcher, and a small patch to Fennel itself that disabled the macro-loaded cache, so that the searcher is always called (so every import passes through the searcher).

rktjmp/hotpot.nvim@f9a4c29

diff --git a/deps/fennel-0.10.0.lua b/deps/fennel-0.10.0.lua
index 9572c34..b18f4dd 100644
--- a/deps/fennel-0.10.0.lua
+++ b/deps/fennel-0.10.0.lua
@@ -2034,7 +2034,7 @@ package.preload["fennel.specials"] = package.preload["fennel.specials"] or funct
     local modexpr = resolve_module_name(ast, scope, parent, {})
     local _ = compiler.assert((modexpr.type == "literal"), "module name must compile to string", (real_ast or ast))
     local modname = load_code(("return " .. modexpr[1]))()
-    if not macro_loaded[modname] then
+    if true or (not macro_loaded[modname]) then
       local env = make_compiler_env(ast, scope, parent)
       local loader, filename = search_macro_module(modname, 1)
       compiler.assert(loader, (modname .. " module not found."), ast)

This PR adds an new hook required-macro that passes the modname, which would let Hotpot use a compiler plugin instead and remove the need to patch Fennel.

I aware this:

• Doesn't conform to the other hooks (does not pass ast or scope), though destructure doesn't either. Perhaps it could pass ast scope modname.
• Doesn't really have the same "feel" as the other hooks, in terms of dealing with more technical aspects.
• Might be have a pretty slim use case, though I image other tools may want to do something similar to what Hotpot does.
• The "tense" of the name might be debated to, it's technically happening before the macro is really there?

Would something like this ever make it into Fennel or is it a bit too off the mark for what the compiler plugins are intended for?

diff --git a/src/fennel/specials.fnl b/src/fennel/specials.fnl
index 819e769..74195ea 100644
--- a/src/fennel/specials.fnl
+++ b/src/fennel/specials.fnl
@@ -1137,6 +1137,7 @@ modules in the compiler environment."
       (let [(loader filename) (search-macro-module modname 1)]
         (compiler.assert loader (.. modname " module not found.") ast)
         (tset macro-loaded modname (loader modname filename))))
+    (utils.hook :required-macro modname)
     (add-macros (. macro-loaded modname) ast scope parent)))
 
 (doc-special :require-macros [:macro-module-name]

[technomancy]

I love this idea, thanks for bringing it up. I think it's a great idea to allow plugins to track dependencies between modules and the macros they load.

I aware this [doesn't] conform to the other hooks (does not pass ast or scope), though destructure doesn't either. Perhaps it could pass ast scope modname.

Different hooks call for different conventions. I would say that all hooks called from special forms ought to pass ast and scope, but for hooks called from the parser or repl it doesn't make any sense. In this case though, we are talking about a special form hook, so for consistency it probably ought to behave like the other special form hooks. Plus if you don't have the AST how do you know which module is being loaded currently and is requiring the macros? At least with the AST you have access to the filename in question.

For the tense, I think :require-macros would be more consistent with the existing hooks.

Please consider implementing this plugin in such a way that it can be used standalone without the rest of Hotpot! I think it could be useful in many other contexts.

[rktjmp]

Took a long time to get back to this.

I added a test, which is pretty basic, there wasn't an explicit hooks test that I could see. Can rename stuff to be more appropriate with guidance.

Passing the AST through, I am only ever seeing filename: "src/fennel/macros.fnl" which isn't so useful. Maybe I misunderstood your comment above, or maybe this is a bug or maybe just a miscommunication.

It does makes sense that the filename might be fennel/macros.fnl since it's happening at compiler time, inside that file (?). Guess I am just checking it's not a bug, I cant tell.

; In my plugin for :require-macros ([ast scope]) ...

fennel.view ast =>
(import-macros "astre.macros.settings")

vim.inspect ast =>
{ { "import-macros",
    filename = "src/fennel/macros.fnl",
    line = 327,
    quoted = true,
    <metatable> = { "SYMBOL",
      __eq = <function 1>,
      __fennelview = <function 2>,
      __lt = <function 3>,
      __tostring = <function 2>
    }
  }, "astre.macros.settings",
  bytestart = 13118,
  filename = "src/fennel/macros.fnl",
  line = 327,
  <metatable> = { "LIST",
    __fennelview = <function 4>,
    __tostring = <function 4>
  }
}

One thing I thought about was naming it require-macros doesn't match import-macros which is the prescribed style:

https://github.com/bakpakin/Fennel/blob/main/reference.md#require-macros-load-macros-with-less-flexibility
The require-macros form is like import-macros, except it does not give you any control over the naming of the macros being imported. It is strongly recommended to use import-macros instead.

Maybe the hook should be :import-macros?

[technomancy]

there wasn't an explicit hooks test that I could see

Try grepping the test dir for plugins and you'll see a few tests we have for this already.

Passing the AST through, I am only ever seeing filename: "src/fennel/macros.fnl"

This is because of an implementation quirk of import-macros; in order to avoid introducing a new special, import-macros simply calls require-macros and does some shuffling around of the return value. The import-macros macro calls the require-macros special form using a specially constructed dummy AST which comes from the macros.fnl file. That explains the filename you're seeing, but this AST is not the right one to expose to the plugin because as you have noted it's not relevant to anything the plugin could want to do.

The way import-macros works, it takes some number of module/binding pairs and loads the macros from each module (using require-macros as an implementation detail), attaching them to the bindings. So for a hook here to make sense, it should either run once for each import-macros invocation and pass in the full list of module/binding pairs, or run once for each module/binding pair. The latter is easier to implement and arguably more useful for plugin authors; it's difficult to imagine a scenario in which it's relevant whether or not module/binding pairs came from the same import-macros call.

Normally the utils.hook function is not exposed to macros. However, the built-in macros are loaded in a special environment which gives them access to some internals that normal macros don't have, including the fennel.utils module exposed as a utils global in that environment. So the hook should probably be added to import-macros itself.

The test you've added is a start, but it would be better if the test could check for something other than "the function in the plugin actually ran or not". Could it be expanded to check that it's able to have some kind of effect on the code which ran, or check that the arguments have at least some connection to the macros in question?