Skip to content

Path/Module resolve callback #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 27, 2024
Merged

Conversation

santiberna
Copy link
Contributor

While working with wrenbind17, I have found the library quite lacking in handling wren imports for files. The original wren library includes a module resolution callback so that things like relative imports and path aliasing problems can be handled correctly. Wrenbind17 does not expose this feature of wren to the user and using the vector of import paths is quite limited.

This pull request fixes that by adding a pathResolveFn to the vm class with an appropriate setter, quite like the printFn and loadModuleFn. This also changes the signature of LoadModuleFn, since it no longer requires the vector of import paths as inputs (since it only needs the resolved output of pathResolveFn).

The default implementation for the path resolution function simply contains the resolution part of the old LoadModuleFn that checks for a valid path based on the import paths. This maintains the behaviour of the library and should not break users that don't want to use this feature.

If I missed anything or you have any tweaks I should consider, please let me know 👍

Copy link
Owner

@matusnovak matusnovak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty good! Looks good to me. Thanks! Merging.

@matusnovak matusnovak merged commit c4ac1d3 into matusnovak:master Dec 27, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants