Olical / aniseed

Neovim configuration and plugins in Fennel (Lisp compiled to Lua)
https://discord.gg/wXAMr8F
The Unlicense
606 stars 28 forks source link

Autoloading with colon-delimited tables is broken #116

Closed sadderchris closed 2 years ago

sadderchris commented 2 years ago

First of all, thanks for making aniseed!

I think I've run into an edge case with the latest commit. Suppose I have a module declaration that looks like this:

(module foo.bar
  {autoload {: cmp}})

;; ...

Previously, this would work just fine and expose cmp as an autoloaded module. Now however, I get a compile error:

Compile error in <user-dir>/fnl/foo/bar.fnl:2
  unable to bind string cmp
  {autoload {: cmp}}

* Try replacing the string cmp with an identifier.

It seems like the binding here is a string in this case, rather than a symbol. Of course, I can work around this by doing something like:

(module foo.bar
  {autoload {cmp cmp}})

;; ...

but this is a bit silly and it would be nice to be able to continue using the shorthand syntax.

(I suspect this isn't just a problem with autoload- it probably affects all table-like bindings in the module declaration, but I've only really encountered it there).

Olical commented 2 years ago

I think this was going to be an issue before the latest destructure support although I'm not sure.

I don't know if this will ever work because it's essentially being translated to this

(module foo
  {require {: xyz
            other some.other.thing})

; =>

(local (: other) (values (require :xyz) (require :some.other.thing))

Which just won't work, the : isn't a valid symbol to bind to. That will only work in a map or let destructure and I can't use that in this context at all (I think it's completely impossible without wrapping all of your code in a let block? Which I can't do from a macro).

Although I just through of a solution while walking through this... I think... if I see a : symbol I need to convert it to the RHS of the assignment, essentially making it sugar for {cmp cmp}... although I don't know if that's possible either. That error isn't coming from Aniseed and it makes me think it's actually coming from the Fennel compiler which just doesn't like the syntax.

I'll have a look though and see if I can trick this into working as you want, it's just harder than it seems because what you see before the macro is a map kind of thing, but after that Aniseed compiles it down to a local with tuple destructuring since it's the only way I can introduce multiple locals with one form.

Olical commented 2 years ago

So if Aniseed's macro receives the : symbol I can maybe convert it to the RHS side of the assignment to get the behaviour you want. If I don't receive it and it's a syntax error in the compiler I don't think there's anything I can do. Will check when I have some free time.

sadderchris commented 2 years ago

Maybe this will be helpful - Here's an example module from my config that I've run through macrodebug:

(macrodebug (module magic.plugin.cmp
  {autoload {{: nil?} aniseed.core
             nvim aniseed.nvim
             : cmp
             : cmp_nvim_lsp
             : cmp-npm
             : lspkind
             : luasnip}
   require-macros [magic.macros]}))
;; ...

Output:

; eval (current-form): (macrodebug (module magic.plugin.cmp {aut...
; (out) ["ANISEED_DELETE_ME"
; (out)  (local *module-name* "magic.plugin.cmp")
; (out)  (local *module* (. package.loaded *module-name*))
; (out)  (local *module-locals* (. *module* "aniseed/locals"))
; (out)  (local autoload (. (require "aniseed.autoload") "autoload"))
; (out)  (local ("cmp" "cmp-npm" "cmp_nvim_lsp" "lspkind" "luasnip" nvim {:nil? nil?} _) (values (autoload "cmp") (autoload "cmp-npm") (autoload "cmp_nvim_lsp") (autoload "lspkind") (autoload "luasnip") (autoload "aniseed.nvim") (autoload "aniseed.core") (require-macros "magic.macros")))
; (out)  (tset *module-locals* "cmp" "cmp")
; (out)  (tset *module-locals* "cmp-npm" "cmp-npm")
; (out)  (tset *module-locals* "cmp_nvim_lsp" "cmp_nvim_lsp")
; (out)  (tset *module-locals* "lspkind" "lspkind")
; (out)  (tset *module-locals* "luasnip" "luasnip")
; (out)  (tset *module-locals* "nvim" nvim)
; (out)  (tset *module-locals* "table: 0x7f56830e7688" {:nil? nil?})
; (out)  (tset *module-locals* "_" _)
; (out)  (local client-capabilities (. *module* "client-capabilities"))
; (out)  (local setup (. *module* "setup"))
; (out)  (local a (. *module-locals* "a"))
; (out)  (local check-back-space (. *module-locals* "check-back-space"))
; (out)  (local cmp (. *module-locals* "cmp"))
; (out)  (local cmp-npm (. *module-locals* "cmp-npm"))
; (out)  (local cmp_nvim_lsp (. *module-locals* "cmp_nvim_lsp"))
; (out)  (local lspkind (. *module-locals* "lspkind"))
; (out)  (local luasnip (. *module-locals* "luasnip"))
; (out)  (local nvim (. *module-locals* "nvim"))]
nil

So it looks like it's not a literal : that's being passed along, but rather the stringified symbol itself (e.g. "cmp" for cmp, but nvim for aniseed.nvim, and of course a table for the nil? binding). I'm wondering if you could just check if bind here is a string and turn it into a symbol if it is, else just pass it along? The "table: 0x7f56830e7688" binding for nil? is also rather weird...

Olical commented 2 years ago

Got it, I understand now. Yeah, that's basically what I'll do 😄

Olical commented 2 years ago

Pushed a fix for this to develop and it's working for me! Just checked if it's a string and converted it to a symbol, good call.

Olical commented 2 years ago

I also expanded on the README example of the module macro syntax to show the destructuring, shorthand and require/autoload difference.

Olical commented 2 years ago

Released!