Olical / aniseed

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

Fix module return value #91

Closed harrygallagher4 closed 2 years ago

harrygallagher4 commented 2 years ago

Continuation of #90

This appears to generate the right lua for my dotfiles. It looks like parinfer changed the files more than necessary so I'll have to fix that tomorrow.

See: https://github.com/harrygallagher4/nvim/blob/95ba7461baf53393bc4f6861d19565b1a940a098/fnl/dotfiles/init.fnl (module that has a return value other than *module*, now with *module* removed) https://gist.github.com/harrygallagher4/847440c11921153d8f4fc6871bb49820#file-init-lua-L34-L38 (compiled lua) https://github.com/harrygallagher4/nvim/blob/2cf6ff701aacb8a5cdf734216c2944c4f3f99804/fnl/dotfiles/test.fnl (non-aniseed module) https://gist.github.com/harrygallagher4/847440c11921153d8f4fc6871bb49820#file-test-lua (compiled)

harrygallagher4 commented 2 years ago

Whoops, accidentally force-pushed without any changes which closed the PR 😅. Sorry if closing/reopening/commenting spams your email.

Anyway the latest commit removes the formatting changes introduced by parinfer. Unfortunately some tests are now failing. From here I'll need some input, if you like these changes then we should probably modify the failing tests. Also one of the tests complains about module not existing so it's possible this can be solved in a smarter way than my hacky macro. I don't have much experience with the fennel compiler so I'll read up and see if I come up with anything.

edit: speaking of that reading... the below commit uses (in-scope (sym "*module*")) instead of naively inserting *module* when it might be nil. As a bonus only one test and one assertion are failing now (and for a somewhat petty reason in my opinion)!

second edit: in-scope actually didn't work for this unfortunately

Olical commented 2 years ago

Will look soon! Away visiting family + working remotely at the moment so will hopefully get time later in the week 😊 thank you for looking into this!

harrygallagher4 commented 2 years ago

So, the latest commit has this at a state that I'm very happy with. Now I'm wrapping the body in a macro that wraps the last expression in another macro which can properly access the scope to check for *module*. I tested it on my dotfiles and everything is working fine, including one module that doesn't use (module ...) which outputs untouched lua. Also, no tests are failing! :)

❯ make test
SUFFIX="test/fnl/foo.fnl" scripts/test.sh
[aniseed.core-test] OK 36/36 tests and 123/123 assertions passed
[aniseed.eval-test] OK 3/3 tests and 36/36 assertions passedfoofoo
bar[aniseed.nvim.util-test] OK 2/2 tests and 7/7 assertions passed
[aniseed.string-test] OK 6/6 tests and 29/29 assertions passed
[aniseed.macros-test] OK 1/1 tests and 3/3 assertions passed
[aniseed.fs-test] OK 3/3 tests and 8/8 assertions passed
[aniseed.compile-test] OK 1/1 tests and 4/4 assertions passed
[total] OK 52/52 tests and 210/210 assertions passed

Also, I went ahead and implemented #66 for fun. https://github.com/harrygallagher4/aniseed/tree/require-additional-macros

No rush on any of this, enjoy your time away :)

Olical commented 2 years ago

I'm making some rename changes locally and reviewing, looking good! I've also wrapped Aniseeds internally bootstrapped files in the same macros so they get the same protection.

Olical commented 2 years ago

Merged and compiled into Conjure's develop branch too! Let's give it a few days to soak and then I think it's probably good to go. So far it seems fine to me!

Olical commented 2 years ago

I really like that it ensures some consistency across ALL files, no more slight chance of returning the wrong table.