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

Allow walking & not-walking the whole quote form #15

Merged
merged 1 commit into from Sep 17, 2014

Conversation

trptcolin
Copy link
Contributor

Input needed on this one.

Usually with riddley.walk/walk-exprs, I can assume I won't walk an expression unless my predicate allows it. I'm using a set, similar to what proteus does. But in the case of quote, I can't avoid walking expressions underneath it.

Originally I thought could just tack a (predicate x) onto the condition in walk-exprs and add another condition that just returns its argument.

But there's more: if you want to be able to walk a quoted form, say to replace quote with foobar, it can't currently be done in master (as far as I can tell), and the naïve in the previous paragraph doesn't solve it. This patch does seem to solve it and pass the tests, but I'm pretty sure it breaks the idea of not macroexpanding inside a quote form. I'm actually having a bit of a hard time writing a test case showing that, but I think I can visualize why it's wrong. So I think sure this isn't really ready for prime time yet, but wanted to get it out there for input & brainstorming.

@immoh
Copy link
Contributor

immoh commented Sep 17, 2014

I don’t think it breaks anything. It could be worth mentioning in the documentation that if you call walk-exprs for the rest of the quoted form, you need to halt macroexpansion, otherwise things break.

For example replacing quote with foobar:

(riddley.walk/walk-exprs #(= 'quote (first %))
                         #(list* 'foobar (riddley.walk/walk-exprs (constantly false) nil (rest %)))
                         '(quote fn))
;; => IllegalArgumentException Parameter declaration missing  clojure.core/fn (core.clj:4089)

This also fixes problem with the current implementation: if you call if you call walk-exprs recursively from the handler function, there’s no way to know if you’re inside quoted form or not and hence whether macroexpansion should be halted or not.

ztellman pushed a commit that referenced this pull request Sep 17, 2014
Allow walking & not-walking the whole quote form
@ztellman ztellman merged commit b147126 into ztellman:master Sep 17, 2014
@ztellman
Copy link
Owner

Ugh, I thought I had already merged this. Sorry.

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.

None yet

3 participants