perf: modularize built-ins #288
Conversation
jose-elias-alvarez
force-pushed
the
modularize-builtins
branch
from
3d93d37 to
b86c1a0
Compare
jose-elias-alvarez
changed the title
jose-elias-alvarez
force-pushed
the
modularize-builtins
branch
from
b86c1a0 to
a1b72dd
Compare
jose-elias-alvarez
force-pushed
the
modularize-builtins
branch
from
a1b72dd to
1aea5fc
Compare
| @@ -1018,7 +1039,7 @@ local sources = { null_ls.builtins.formatting.shfmt } | |||
|
|
|||
| ##### Defaults | |||
|
|
|||
| - `filetypes = { "sh" }` | |||
| - `filetypes = { "sh", "zsh" }` | |||
There was a problem hiding this comment.
I am not sure if shfmt supports zsh actually.
mvdan/sh#120
There was a problem hiding this comment.
That's strange – I've been using shfmt for zsh since we added it about 2 weeks ago and haven't run into any problems. I don't know much about the project, but maybe the issue you linked is about a different feature?
There was a problem hiding this comment.
Hmm, not so sure, I just tried it myself for the first time and it reformatted:
if (( $+commands[ls] )); thento
if (($ + commands[ls])); thenwhich is not valid and errors with bad math expression. There is no mention of official support for zsh in shfmt's repository, so I would recommend not setting it as default option here.
There was a problem hiding this comment.
Got it – I'll remove it from the defaults.
There was a problem hiding this comment.
Thanks, amazing refactor btw, excellent work on null-ls, using it every day!
The library of built-ins has grown beyond what I imagined. Currently, built-ins are modularized by method, which means that requiring and registering a single built-in runs
helpers.make_builtinonce for every other built-in for the same method. The performance impact isn't huge, but it'll naturally grow bigger as more built-ins are added, and working with built-in files is also becoming tedious.This PR lays out a simple structure to completely modularize built-ins using metatables. Finishing this will require a couple of hours of copy-pasting but shouldn't be too difficult.