Make hooks funcallable.

[aartaka]

This makes hooks funcall-able. Not much practical value, except allowing for more relaxed hook invocations, compared to the regular nhooks:run-hook.

This adds a closer-mop dependency, which might be problematic for other projects (potentially) relying on this library, but it's fine for Nyxt—it already depends on closer-mop :)

As an additional mind-boggling thing: SBCL allows extending sequence class to allow for any class to behave like sequences. Maybe implement this too, indexing handlers etc...

Does that look sane?

[aartaka]

@Ambrevar, any feedback?

[Ambrevar]

This adds a closer-mop dependency, which might be problematic for other projects (potentially) relying on this library

closer-mop could have been named trivial-mop: it's super lightweight, no dependencies. So no hassle, we can include it!

[aartaka]

OK, handlers are funcallable now. Which makes me wonder: do we actually need fn anymore?—it all can be simulated with funcall-able instance setting. Not sure how symbol-based handlers would work, but we can generate proxy lambdas for those in the worst case ( ͡° ͜ʖ ͡°)

[Ambrevar]

I'm not sure we can get rid of fn because how would we check the types otherwise?

[aartaka]

I'm not sure we can get rid of fn because how would we check the types otherwise?

We can do that the same way we do it now: by compiling and ftype-declaring it. For example, the following two snippets work as intended (on SBCL, CCL, and ECL):

;; Basic type check. Handlers are functions.
(typep (make-instance 'nhooks:handler :name 'hey :fn (lambda () (print "hey!"))) 'function)
;; A more complicated check from nhooks::probe-ftype. Handlers inherit the type of their funcallable instances.
(compile 
 nil
 `(lambda ()
    (let ((fn ,(make-instance 'nhooks:handler :name 'hey :fn (lambda () (print "hey!")))))
      (declare (type (function ()) fn))
      fn)))

[aartaka]

CL is so well-planned...

[Ambrevar]

I wasn't sure it also works on values... Just checked: it does.

From what I recall it does not work on CCL because the compiler does less function type checking, plus it does not raise a condition.

Did you get it to work on CCL?

Also you are still using :fn. Or do you mean to only use it as an initialize-instance :after parameter?

[aartaka]

I wasn't sure it also works on values... Just checked: it does.

From what I recall it does not work on CCL because the compiler does less function type checking, plus it does not raise a condition.

Did you get it to work on CCL?

I did, but note that I've checked to arg-less function type, and it may stop working if we add args...

Also you are still using :fn. Or do you mean to only use it as an initialize-instance :after parameter?

I am using it, but we can safely turn it into an initialize-instance :after parameter and closer-mop:set-funcallable-instance-function on it there!

[Ambrevar]

Exactly, CCL cannot go as sophisticated as SBCL. Try the following on various compiler:

(compile 
 nil
 `(lambda ()
    (let ((fn (lambda (s) (* 2 s))))
      (declare (type (function (number) string) fn))
      fn)))

(compile 
 nil
 `(lambda ()
    (let ((fn (lambda (s) (declare (type number s)) (* 2 s))))
      (declare (type (function (string) number) fn))
      fn)))

[aartaka]

But then, does it work right now? What would incorporating functions inside handlers change anyway?

[Ambrevar]

Sorry, what works right now?

[aartaka]

I mean, handler/hook ftype checking on CCL. Your snippet applies to functions, which makes me think that it doesn't work as a super-reliable ftype checker, at least on CCL. But then, you've used it as an argument against :fn slots being merged inside handlers, somewhy saying that situation with ftype checking would change in some way if we merge those.

But it won't -- CCL ftype checking would still be too permissive, no matter what we're checking -- functions or some funcallable-standard-object. It should be fine merging fn inside the handler, as it will change nothing except removing an unnecessary slot from the semantically funcallable and ftype-check-able object.

[Ambrevar]

Misunderstanding: I was just being confused because nhooks used to be implemented differently, by type-checking functions. But that does not make sense with the current implementation.

All good!

[aartaka]

Okay, there's no more fn. However, all this funcallability business strips us off the most important feature of nhooks -- binding handlers to symbols, with the ability of symbol-bound function redefinition.

Given that, we'd have to revert the handler funcallability so that handlers use the old mechanism. But then, it wouldn't make much sense making hooks solely funcallable, I guess?

I can see the benefits of both approaches (making everything funcallable vs. allowing symbol hooks), but I guess allowing symbols is more pragmatic. @Ambrevar, @BlueFlo0d, @aadcg, any ideas on that? Can we have both?

Unless anyone is strongly against it, or unless we find some way to support symbols, I'll close this PR in a week :)

[Ambrevar]

Err... can you give an example? (Sorry, I haven't touched this lib in a while...)

[aartaka]

Look at this code, for example. If we use a symbol as fn of a hanldler, it can change at arbitrary moment and the version of nhooks before this PR would handler this, using the new value of a symbol-designated function, but this PR would make it unable to change:

(defun hello () (print "hey!"))
(defvar handler (make-instance 'nhooks:handler :fn 'hello))
(defun hello () (print "hola!"))
;; Before this PR:
(funcall (nhooks:fn handler))
=> "hola!"
;; As per this PR
(funcall handler)
=> "hey!"

[Ambrevar]

Oh yes, that makes sense.

Having funcallable hooks and handlers is still possible even if we keep the fn slot though, no?

[aartaka]

Yes, that would keep the best of both worlds. Let me think on how to approach that properly...

[aartaka]

OK, the old behavior is preserved (in regards to symbol-bound handlers), while both hooks and handlers are properly funcallable. It's fully backward-compatible. I'll think some more on how to make it more elegant, and will merge today's evening.

[aadcg]

Interesting work. Nothing much to add :)

[Ambrevar]

Thanks!