DarkWiiPlayer / glass

Simple config loading for Lua
The Unlicense
4 stars 1 forks source link

would it be feasible to add a loader for fennel-files #1

Open erikLundstedt opened 2 months ago

erikLundstedt commented 2 months ago

I currently just require("config-file") but that feels unsafe for a config file

fennel: https://fennel-lang.org

erikLundstedt commented 2 months ago

PS: I might end up making a module myself, but I am not the best at "vanilla" lua, as I mostly write fennel-code

PPS: this would probably be almost the exact same as the lua module, or a module to load moonscript.

PPPS: this seems like a cool project

DarkWiiPlayer commented 2 months ago

It should be about as easy as doing this

return function(name)
    local f = load(fennel.compile(name..'.fnl'))
    return f and f() or nil
end

Feel free to open a PR if you want to play around with it, otherwise I'll try whenever I get around to it

erikLundstedt commented 2 months ago

I have managed to make something that seems to work, but it needs more testing,especially as I wrote it in fennel and have been testing it in fennel...(I did transpile it though)

https://github.com/erikLundstedt/glass-fennel-loader

ps: I followed you on mastodon, I'm @erilun06@vivaldi.social

erikLundstedt commented 2 months ago

https://gitlab.com/fennel-scripts/glasshouse is my testing repo

install fennel: luarocks install fennel --local and then just run ./src/main.fnl it should output the data in app/config/settings.fnl after evaluating any functions

you can safely ignore the bots.fnl and project.fnl files as they are part of my build(or rather "code management") system :-)

my module does NOT allow for returning a "raw" function, but you can return a function call

(fn config []
  {:foo "bar"
   :latin ["lorem" "ipsum"]
   :names ["alice" "bob"]})
(config)  ;;return config()
;;works -> {:foo "bar" :latin ["lorem" "ipsum"] :names ["alice" "bob"]}

works but not recommended for config-style files

  {:foo "bar"
   :latin ["lorem" "ipsum"]
   :names ["alice" "bob"]}
;;works -> {:foo "bar" :latin ["lorem" "ipsum"] :names ["alice" "bob"]}

this is the recommended way of using it, makes it read more like json than actual code

(fn config []
  {:foo "bar"
   :latin ["lorem" "ipsum"]
   :names ["alice" "bob"]})
;;will not work-> ;;<function 23192398123098123>

this returns the config function, my module won´t execute it

definetly not recomended

erikLundstedt commented 2 months ago

I could add a type-check to make sure the config returns a non-function

DarkWiiPlayer commented 2 months ago

Is there anything wrong with just returning raw functions? That seems like it could be useful, even when loading fennel modules from Lua

erikLundstedt commented 2 months ago

Is there anything wrong with just returning raw functions? That seems like it could be useful, even when loading fennel modules from Lua

Its not recommended according to the fennel style guidelines(and I got an error when trying to port the lua module)

https://fennel-lang.org/style#modules

Always return a table from a module. Even if you think today that returning a bare function is fine, you will regret it later.

Loading a module should have no side effects. Since modules are tables, their contents can be changed at runtime. Avoid this temptation, except in the case of reloading the entire module.

The best alternative is probably to put the function in the returned table, Which definitely works but at the point it's strictly needed, you should probably just use the require feature of fennel/lua (That's just my opinion though)

erikLundstedt commented 2 months ago

Looking at the fennel "configs" as a lispy json (no returned functions, just return a table with data) also means you can "generate" and "update" the config files using fennel.view (which does what toString should do when given a table)

DarkWiiPlayer commented 1 month ago

Its not recommended according to the fennel style guidelines(and I got an error when trying to port the lua module)

That's not bad advise for modules, but it doesn't seem like a thing glass needs to enforce for the user. On the contrary, in order to really be as transparent as possible, it shouldn't make a difference whether foo.bar() loads a file foo.lua that returns a table with a foo element, or a file foo/bar.lua that directly returns a function.

Anything too complex should probably be a proper module anyway, but that's also not something a library like glass should be enforcing.

erikLundstedt commented 1 month ago

I just confirmed, it works to just return a function, its not "supported" by me though as it may require special handling on the program side to check if the "loaded" config is actually a "raw" function

erikLundstedt commented 1 month ago

do you have any other notes on my code/module? I´m not too familiar with the thing you are doing in the spec folder, so no effort has been made there on my part

DarkWiiPlayer commented 1 month ago

do you have any other notes on my code/module? I´m not too familiar with the thing you are doing in the spec folder, so no effort has been made there on my part

The tests are all written in moonscript for convenience and run using busted

If you want to test your module, you can throw a bunch of fennel files in spec/fixtures/fennel and create a fennel_loader_spec.lua file with your tests. They're a bit more verbose in Lua, but still very manageable.


do you have any other notes on my code/module?

I'm not entirely sure if calling fennel.install in the module loader makes sense.

When using glass from within fennel, it should already be installed anyway, so just calling fennel.dofile() directly should be enough.

Loading fennel files from Lua seems like a weird special case. I don't see a clear use-case for that, so I think it's acceptable to say fennel configs can't require fennel modules unless the application author manually calls fennel.install() elsewhere in their code.


It's a small requirement that keeps a possibly unexpected side effect out of the loader.

Returning cfg or {} also seems like a pointless artificial limitation to me; it lets you return true from a config file but neither false nor nil.


The generated Lua code already looks good except for two minor details:

  1. The file is indented with spaces instead of tabs
  2. The function is saved into a temporary _1_ variable instead of being returned directly
erikLundstedt commented 1 month ago

I'm not entirely sure if calling fennel.install in the module loader makes sense.

When using glass from within fennel, it should already be installed anyway, so just calling fennel.dofile() directly should be enough.

the fennel.install thing is taken directly from the fennel docs: https://fennel-lang.org/api#evaluate-a-file-of-fennel I just made it multiple lines for my own sanity.

  1. The file is indented with spaces instead of tabs.

I agree with that point, I'll add sed to the transpile script.

  1. The function is saved into a temporary 1 variable instead of being returned directly

this is a quirk of the fennel transpiler, there is not a lot I can really do about it other than hand-editing the generated lua-code. and it does work the same, right.

Returning cfg or {} also seems like a pointless artificial limitation to me; it lets you return true from a config file but neither false nor nil.

this is just me trying to be nil-safe, and wanting to at least return a table(again, something I try to do when writing fennel code), I could change it to return cfg or false but that makes it impossible to return nil, which you might not want

it was meant as a safeguard for if the file cant be found, but was a bit naive

My next git push will have return cfg if you dont have any better ideas

DarkWiiPlayer commented 1 month ago

If you're going to create a PR, I would suggest just working in fennel until you get the result you want, and then clean up the Lua file by hand at the end. For such a simple bit of code, I don't see much of a reason to keep the original fennel code around in the project, specially given how neatly it compiles to Lua.

Nil-safety isn't really a major concern here, as a module returning nil would just be considered "not found" by the loader and thus skipped. If this was a problem, I feel like the right way to handle it would be to raise an error, rather than to return a default value.

Any projects with stricter requirements should probably ship its own loader and pass that to the glass wrapper directly.

erikLundstedt commented 1 month ago

If you're going to create a PR, I would suggest just working in fennel until you get the result you want, and then clean up the Lua file by hand at the end. For such a simple bit of code, I don't see much of a reason to keep the original fennel code around in the project, specially given how neatly it compiles to Lua.

Nil-safety isn't really a major concern here, as a module returning nil would just be considered "not found" by the loader and thus skipped. If this was a problem, I feel like the right way to handle it would be to raise an error, rather than to return a default value.

Any projects with stricter requirements should probably ship its own loader and pass that to the glass wrapper directly.

I will keep/put the fennel source in my "glasshouse" project (and put that code as mit/same as this Repo, the rest is simpl/GPL V2)(that is where i am playing around with the code)

I can definitely go for pushing only the lua code in the future merge-request

I have personally started using my fork in a couple projects that has been using the built-in require() previously

Working towards being able to remove .config/fennelscripts/... from my fennel path 😃