risor-io / risor

Fast and flexible scripting for Go developers and DevOps.
https://risor.io
Apache License 2.0
581 stars 24 forks source link

feat: allow excluding builtins #149

Closed luisdavim closed 6 months ago

luisdavim commented 6 months ago

relates to: #89

myzie commented 6 months ago

To fully remove the denied builtins, it seems like we need to have a mechanism to remove them from the Config DefaultGlobals and Globals maps. Including removing them from the Module objects stored there, for module-provided builtins.

(This is just based on a quick look, see also my comment here)

luisdavim commented 6 months ago

OK, I'll have another look at this, thanks.

luisdavim commented 6 months ago

Updated the PR with another approach, deleting top level builtin from the config and overriding the module provided builtin with one that always returns an error, this seems to work, at least for my use cases, let me know what you think.

myzie commented 6 months ago

@luisdavim - I riffed on what you were doing here: https://github.com/risor-io/risor/pull/151/files

Would that way of doing it work for your purposes? If generally yes, we could add WithGlobalOverride to that if you still need it in addition to what I did there.

luisdavim commented 6 months ago

@luisdavim - I riffed on what you were doing here: https://github.com/risor-io/risor/pull/151/files

Would that way of doing it work for your purposes? If generally yes, we could add WithGlobalOverride to that if you still need it in addition to what I did there.

I think the WithGlobalOverride will be useful to me as well, do you see anything wrong with my approach? Not sure why you opened that separate PR but I'll take a closer look and check the differences.

luisdavim commented 6 months ago

@luisdavim - I riffed on what you were doing here: https://github.com/risor-io/risor/pull/151/files

Would that way of doing it work for your purposes? If generally yes, we could add WithGlobalOverride to that if you still need it in addition to what I did there.

I think the WithGlobalOverride will be useful to me as well, do you see anything wrong with my approach? Not sure why you opened that separate PR but I'll take a closer look and check the differences.

OK, I see, I do like the support for nested modules 👍 but I do think WithGlobalOverride will be useful to me so if you could add it there, that would be great. Thanks.

luisdavim commented 6 months ago

closing in favour of #151.