-
Notifications
You must be signed in to change notification settings - Fork 66
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
Ability to add additional superclasses to a sketch. #151
base: master
Are you sure you want to change the base?
Conversation
Interesting! Have you considered making it backwards compatible by creating something like (defsketch foo ((mixins (a b c)))
(do-stuff)) ? |
I have considered that, might be nicer than having an entirely separate interface. Another possibility:
The only tricky part would be testing whether theres an options list, I guess it should be backward compatible to test whether the first item in the body is a plist. |
But that's still not backwards compatible, is it? I mean I think just using mixins with a list works ok |
What about (defsketch foo ((:mixins (a b c))
(some-variable 'hello))
(do-stuff)) or maybe just (defsketch foo ((:mixins a b c)
(some-variable 'hello))
(do-stuff)) By using a keyword name we show that this parameter is different from usual slots; but the API is kept backward compatible and uniform. |
@Gleefre what are the cons of using the non-keyword version? is it about the need to figure out what happens when you change it from the code? |
I might be wrong, but I think my version is backward compatible? The bindings would still be there...
And the code-generating logic would check whether the first item after the name is a plist. If so, treat it as the
I'm also open to making the configuration options part of the bindings list, of course. Depends on which option ye think is neater. And yeah, I think the point of using keyword args is that they're easily distinguishable from regular bindings and should be backward compatible in case someone happens to have a slot called |
3b54fb3
to
f57a658
Compare
I went with your first suggestion, @Gleefre ! Now options like My example usage is unchanged, except it now looks like:
As expected, it generates the following code:
Oh, I forgot to update the documentation. |
It is an interesting question what should happen if mixin list is changed... Dunno. My point of using keyword is to make it more readable -- it hints that this is not a slot (since you can't have variables named by keywords); but an additional option to the macro itself. |
As suggested by Gleefre, the :mixins binding form now looks like (:mixins m1 m2) instead of (:mixins (m1 m2)), 'cause it looks neater. Also, it can appear anywhere in the list of binding forms, not just at the start. (sketch-slot-value 'slot-name) can be used to access slots that are introduced by the mixin. Previously, I'd exported *sketch* so that users could do (slot-value *sketch* 'slot-name), but that seems to expose too much implementation detail.
Updated! Copying the commit message to describe the change. As suggested by Gleefre, the
|
First draft. Adds a
defsketchx
macro that allows specifying additional superclasses for a sketch, which means that behaviour can be shared between sketches and it's easier to develop extensions for sketch.(As per: #142).
Usage example:
I've also tested out sharing state between a sketch and its superclass, e.g. accessing a superclass slot from the sketch class, and it seems to work as expected, though this requires the user to be know that they can do
(slot-value *sketch* 'slot-name)
in the sketch body. I don't think that's documented, and I don't think it would work outside thesketch
package because*sketch*
is not exported.