atlas-engineer / nclasses

A `define-class` macro for less boilerplate
Other
7 stars 2 forks source link

Add `make-instance*` convenience macro #12

Closed aartaka closed 1 year ago

aartaka commented 1 year ago

This adds make-instance*, a helper for make-instance, abstracting over the frequent uses of make-instance encountered in Nyxt and elsewhere.

Addresses https://github.com/atlas-engineer/nyxt/issues/2584

Features

Shortcut args to abstract eponymous keyword arguments:

(make-instance* 'command (name lambda-expression visibility))
=>
(make-instance 'command :name name :lambda-expression lambda-expression :visibility visibility)

And the last argument being an arbitrary form to apply to:

(make-instance* 'scheme (callback) :name scheme-name keys)
=>
(apply #'make-instance 'scheme :callback callback :name scheme-name keys)

Discussion

Given that we start adding more and more CLOS wrappers to nclasses, maybe we should make the naming more consistent by adding aliases like:

I don't suggest removing the existing ones, I just suggest adding wrappers around them. Even if we don't use these, having a consistent set of macros won't hurt.

Ambrevar commented 1 year ago

Sorry, this fell under my radar because GitHub didn't notify me...

Ambrevar commented 1 year ago

So if I get it right, the main goal of make-instance* is to avoid the cumbersome idiom for when we only want to instantiate non-nil slots:

(apply 'make-instance 'foo-class :a a-value (when b-value `(:b ,b-value)))

and replace it with just

(make-instance 'foo-class (a-value b-value))

If so, I think you should make this more explicit in the docstring.

aartaka commented 1 year ago

So if I get it right, the main goal of make-instance* is to avoid the cumbersome idiom for when we only want to instantiate non-nil slots:

No, that's not the case. It was a thought in the initial draft, but that was too involved and easy to shoot feet off with.

(apply 'make-instance 'foo-class :a a-value (when b-value `(:b ,b-value)))

and replace it with just

(make-instance 'foo-class (a-value b-value))

No, make-instance* does only two things:

So, so have a length form like the one you list above, one has to write:

(make-instance* 'foo-class (a) (when b-value `(:b ,b-value)))
=>
;; Notice the :a a, because (a-value) would expand to :a-value a-value!
(apply 'make-instance 'foo-class :a a (when b-value `(:b ,b-value)))

It's not shortening much for small number of args, but is significantly helpful for 3+ args eponymous to slot names.

aartaka commented 1 year ago

Oooops, seems like hard-reseting the branch closes the PR :D

I've refactored the logic one more time, now it should be reliable enough (or emit warnings on cases where it can't) :D

aartaka commented 1 year ago

Merged. Will document it on main and will send the aliases as a separate PR.