Skip to content
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

Add a way to use currently defined reader macros with hy.read-many and hy.read #2292

Closed
Kodiologist opened this issue May 25, 2022 · 4 comments · Fixed by #2494
Closed

Add a way to use currently defined reader macros with hy.read-many and hy.read #2292

Kodiologist opened this issue May 25, 2022 · 4 comments · Fixed by #2494

Comments

@Kodiologist
Copy link
Member

Currently there is no (official) way to read code that uses reader macros defined by the active reader.

@Kodiologist Kodiologist changed the title Add a way to get the current reader for use with hy.read-many and hy.read Add a way to use currently defined reader macros with hy.read-many and hy.read Jun 10, 2022
@scauligi
Copy link
Member

This currently works:

(defreader foo 3)
(setv reader (hy.reader.HyReader))

;; import all readers from current module into reader
(hy.eval (hy.read f"(require {__name__} :readers *)" :reader reader))

(hy.read "#foo" :reader reader)

A slightly more direct solution is:

(hy.macros.enable-readers None reader "ALL")

since that's the only part of require we actually need.

Is enable-readers considered part of the public API?

@Kodiologist
Copy link
Member Author

Kodiologist commented Aug 13, 2023

I'd say it isn't, currently. Originally Allie had intended it to be, the idea being that reader macros can be surprising, and so by calling enable-readers explicitly the user would be opting into reader macros. I argued that (defreader …) or (require foo :readers …) was enough of an opt-in. So in her final version of the PR, enable-readers no longer had to be called by the user, but it remained part of the implementation.

@scauligi
Copy link
Member

I can make a PR to clean up and expose enable-readers for this use-case; the only caveat is that since (like many of the functions from hy/macros.py) it uses inspect.stack()[1] to get the module of the immediate caller, which can lead to potential confusion on the off chance a user is wrapping/calling this indirectly from a utility submodule or something. I don't have a solution to that aside from making sure it's documented, so hopefully that will suffice.

@Kodiologist
Copy link
Member Author

Maybe hy.read could have a special parameter that would enable loading all reader macros from the current module first. Or it could be a parameter of the HyReader constructor.

…it uses inspect.stack()[1] to get the module of the immediate caller, which can lead to potential confusion on the off chance a user is wrapping/calling this indirectly from a utility submodule or something. I don't have a solution to that aside from making sure it's documented, so hopefully that will suffice.

Yes, I think just documenting it should suffice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants