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

Update advice system to not require defining advisable functions #168

Open
jaidetree opened this issue Aug 20, 2022 · 1 comment
Open

Update advice system to not require defining advisable functions #168

jaidetree opened this issue Aug 20, 2022 · 1 comment

Comments

@jaidetree
Copy link
Collaborator

There was a fennel online meetup, and I gave an impromptu overview of spacehammer and hammerspoon and went over the advising system.

Technomancy suggested another approach: Apply the advice at a module level to replace the target functions. This means defadvice would take an extra arg for module location but it could update the module registry at runtime and it would not need any make-advisable or defn macros.

@Grazfather
Copy link
Collaborator

Trying to avoid reading all the code right now, but iirc the way we do it is we define the function as advisable, and it keeps a meta table that remembers if its advised, and if so, knows to invoke the advised functions when and where.

I like his suggestion, it's certainly simpler. Only thing I can think of as a trade-off is that if a handle to the function directly were held anywhere, then advising the function wouldn't 'apply' there.

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

No branches or pull requests

2 participants