From 1fa6ed4e5f1eee91a1da5b1cd6f80d4a8931f468 Mon Sep 17 00:00:00 2001 From: Nathaniel Nicandro Date: Fri, 17 Apr 2020 13:07:03 -0500 Subject: [PATCH] Remove the `jupyter-ioloop-handler` method Replace it with a handler function passed to `jupyter-ioloop-start`. * jupyter-ioloop-comm.el: Do it. * jupyter-channel-ioloop-comm.el: Do it. * jupyter-channel-ioloop-comm.el: Do it. * jupyter-ioloop.el: Do it. Update comments and doc. (jupyter-ioloop--delete-process): Remove. It was used to perform cleanup when the object used for dispatching to a handler method was garbage collected. Since we no longer rely on having an object for dispatching, preferring just a function to call, its not needed. (jupyter-ioloop--make-filter): Fall back to calling handler function instead of `jupyter-ioloop-handler`. Only handle ioloop start/stop events internally. This means we can remove the check for those in `jupyter-ioloop-comm`. * test/jupyter-test.el: Update tests to take into account above changes. --- jupyter-channel-ioloop-comm.el | 30 ++++++++--------- jupyter-ioloop-comm.el | 12 ++----- jupyter-ioloop.el | 60 ++++++++++++---------------------- test/jupyter-test.el | 24 +++++++------- 4 files changed, 48 insertions(+), 78 deletions(-) diff --git a/jupyter-channel-ioloop-comm.el b/jupyter-channel-ioloop-comm.el index 28d4ba89..40b51152 100644 --- a/jupyter-channel-ioloop-comm.el +++ b/jupyter-channel-ioloop-comm.el @@ -90,7 +90,19 @@ (with-slots (ioloop session) comm (unless (jupyter-ioloop-alive-p ioloop) (jupyter-channel-ioloop-set-session ioloop (oref comm session)) - (cl-call-next-method)) + (jupyter-ioloop-start + ioloop (lambda (event) + (pcase (car event) + ;; These channel events are from `jupyter-channel-ioloop' + ('start-channel + (setf (jupyter-proxy-channel-alive-p + (plist-get (oref comm channels) (cadr event))) + t)) + ('stop-channel + (setf (jupyter-proxy-channel-alive-p + (plist-get (oref comm channels) (cadr event))) + nil)) + (_ (jupyter-event-handler comm event)))))) (cl-loop for channel in '(:hb :shell :iopub :stdin) do (jupyter-start-channel comm channel)))) @@ -101,22 +113,6 @@ do (jupyter-stop-channel comm channel)) (cl-call-next-method)) -;;;; `jupyter-channel-ioloop' events - -(cl-defmethod jupyter-ioloop-handler ((_ioloop jupyter-channel-ioloop) - (comm jupyter-channel-ioloop-comm) - (event (head stop-channel))) - (setf (jupyter-proxy-channel-alive-p - (plist-get (oref comm channels) (cadr event))) - nil)) - -(cl-defmethod jupyter-ioloop-handler ((_ioloop jupyter-channel-ioloop) - (comm jupyter-channel-ioloop-comm) - (event (head start-channel))) - (setf (jupyter-proxy-channel-alive-p - (plist-get (oref comm channels) (cadr event))) - t)) - ;;;; Channel querying methods (cl-defmethod jupyter-comm-alive-p ((comm jupyter-channel-ioloop-comm)) diff --git a/jupyter-ioloop-comm.el b/jupyter-ioloop-comm.el index 6b4ef0a2..e272a33e 100644 --- a/jupyter-ioloop-comm.el +++ b/jupyter-ioloop-comm.el @@ -36,21 +36,15 @@ ((ioloop :type jupyter-ioloop)) :abstract t) -;; Fall back method that catches IOLoop events that have not been handled by -;; the communication layer already. -(cl-defmethod jupyter-ioloop-handler ((_ioloop jupyter-ioloop) - (comm jupyter-ioloop-comm) - event) - (unless (memq (car event) '(start quit)) - (jupyter-event-handler comm event))) - (cl-defmethod jupyter-send ((comm jupyter-ioloop-comm) &rest event) (apply #'jupyter-send (oref comm ioloop) event)) (cl-defmethod jupyter-comm-start ((comm jupyter-ioloop-comm)) (with-slots (ioloop) comm (unless (jupyter-ioloop-alive-p ioloop) - (jupyter-ioloop-start ioloop comm)))) + (jupyter-ioloop-start + ioloop (lambda (event) + (jupyter-event-handler comm event)))))) (cl-defmethod jupyter-comm-stop ((comm jupyter-ioloop-comm)) (with-slots (ioloop) comm diff --git a/jupyter-ioloop.el b/jupyter-ioloop.el index 5f25fed3..f4349b8f 100644 --- a/jupyter-ioloop.el +++ b/jupyter-ioloop.el @@ -30,21 +30,19 @@ ;; environment. You add an event that can be handled in the ioloop environment ;; by calling `jupyter-ioloop-add-event' before calling `jupyter-ioloop-start'. ;; -;; In the event handler of the ioloop, you may optionally return another event -;; back to the parent process. In this case, when the parent process receives -;; the event it is dispatched to an appropriate `jupyter-ioloop-handler'. +;; When one of the events added through `jupyter-ioloop-add-event' +;; returns something other than nil, it is sent back to the parent +;; process and the handler function passed to `jupyter-ioloop-start' +;; is called. ;; ;; An example that will echo back what was sent to the ioloop as a message in ;; the parent process: ;; -;; (cl-defmethod jupyter-ioloop-handler ((ioloop jupyter-ioloop) (tag (eql :tag1)) (event (head echo))) -;; (message "%s" (cadr event))) -;; ;; (let ((ioloop (jupyter-ioloop)) ;; (jupyter-ioloop-add-event ioloop echo (data) ;; "Return DATA back to the parent process." ;; (list 'echo data)) -;; (jupyter-ioloop-start ioloop :tag1) +;; (jupyter-ioloop-start ioloop (lambda (event) (message "%s" (cadr event)))) ;; (jupyter-send ioloop 'echo "Message") ;; (jupyter-ioloop-stop ioloop)) @@ -128,10 +126,10 @@ listen for stdin/socket events using `jupyter-ioloop-poller'. You add events to be handled by the subprocess using `jupyter-ioloop-add-event', the return value of any event added is what is sent to the parent Emacs process and what will -eventually be used as the EVENT argument of -`jupyter-ioloop-handler', which see. To suppress the subprocess -from sending anything back to the parent, ensure nil is returned -by the form created by `jupyter-ioloop-add-event'. +eventually be the sole argument to the handler function passed to +`jupyter-ioloop-start'. To suppress the subprocess from sending +anything back to the parent, ensure nil is returned by the form +created by `jupyter-ioloop-add-event'. See `jupyter-channel-ioloop' for an example of its usage.") @@ -143,15 +141,6 @@ See `jupyter-channel-ioloop' for an example of its usage.") (when (process-live-p process) (delete-process process)))))) -(cl-defgeneric jupyter-ioloop-handler ((_ioloop jupyter-ioloop) obj event) - "Define a new IOLOOP handler, dispatching on OBJ, for EVENT. -OBJ will be the value of the object passed to -`jupyter-ioloop-start' and EVENT will be an event as received by -a filter function described in `zmq-start-process'." - ;; Don't error on built in events - (unless (memq (car-safe event) '(start quit)) - (error "Unhandled event (%s %s)" (type-of obj) event))) - (defun jupyter-ioloop-wait-until (ioloop event cb &optional timeout progress-msg) "Wait until EVENT occurs on IOLOOP. If EVENT occurs, call CB and return its value if non-nil. CB is @@ -342,13 +331,6 @@ nothing." (zmq-poller-remove jupyter-ioloop-poller socket) (cl-decf jupyter-ioloop-nsockets))) -(defun jupyter-ioloop--delete-process (process) - (when-let* ((stdin (process-get process :stdin)) - (socket-p (zmq-socket-p stdin))) - (zmq-close stdin) - (process-put process :stdin nil)) - (delete-process process)) - (defun jupyter-ioloop--body (ioloop on-stdin) `(let (events) (condition-case nil @@ -414,26 +396,25 @@ polling the STDIN file handle." (with-slots (process) ioloop (and (process-live-p process) (process-get process :start)))) -(defun jupyter-ioloop--make-filter (ioloop ref) +(defun jupyter-ioloop--make-filter (ioloop handler) (lambda (event) - (with-slots (process) ioloop + (let ((process (oref ioloop process))) + (process-put process :last-event event) (cond ((eq (car-safe event) 'start) (process-put process :start t)) ((eq (car-safe event) 'quit) - (process-put process :quit t))) - (process-put process :last-event event)) - (let ((obj (jupyter-weak-ref-resolve ref))) - (if obj (jupyter-ioloop-handler ioloop obj event) - (jupyter-ioloop--delete-process (oref ioloop process)))))) + (process-put process :quit t)) + (t + (funcall handler event)))))) (cl-defgeneric jupyter-ioloop-start ((ioloop jupyter-ioloop) - object + handler &key buffer) "Start an IOLOOP. -OBJECT is an object which is used to dispatch on when the current -Emacs process receives an event to handle from IOLOOP, see -`jupyter-ioloop-handler'. +HANDLER is a function of one argument and will be passed an event +received by the subprocess that IOLOOP represents, an event is +just a list. If IOLOOP was previously running, it is stopped first. @@ -465,8 +446,7 @@ the IOLOOP subprocess buffer, see `zmq-start-process'." ;; scope, the ioloop process should be killed off. This ;; wouldn't happen if we hold a strong reference to ;; OBJECT. - :filter (jupyter-ioloop--make-filter - ioloop (jupyter-weak-ref object)) + :filter (jupyter-ioloop--make-filter ioloop handler) :buffer buffer))) (oset ioloop process process) (when stdin diff --git a/test/jupyter-test.el b/test/jupyter-test.el index 2ab229e7..b3d831ad 100644 --- a/test/jupyter-test.el +++ b/test/jupyter-test.el @@ -961,7 +961,7 @@ (let ((ioloop (jupyter-ioloop)) (jupyter-default-timeout 2)) (should-not (process-live-p (oref ioloop process))) - (jupyter-ioloop-start ioloop :tag1) + (jupyter-ioloop-start ioloop #'ignore) (should (equal (jupyter-ioloop-last-event ioloop) '(start))) (with-slots (process) ioloop (should (process-live-p process)) @@ -973,17 +973,17 @@ (defvar jupyter-ioloop-test-handler-called nil "Flag variable used for testing the `juyter-ioloop'.") -(cl-defmethod jupyter-ioloop-handler ((_ioloop jupyter-ioloop) - (_tag (eql :test)) - (event (head test))) - (should (equal (cadr event) "message")) - (setq jupyter-ioloop-test-handler-called t)) +(defun jupyter-test-ioloop-start (ioloop) + (jupyter-ioloop-start + ioloop (lambda (event) + (should (equal (cadr event) "message")) + (setq jupyter-ioloop-test-handler-called t)))) (ert-deftest jupyter-ioloop-wait-until () :tags '(ioloop) (let ((ioloop (jupyter-ioloop))) (should-not (jupyter-ioloop-last-event ioloop)) - (jupyter-ioloop-start ioloop :test) + (jupyter-test-ioloop-start ioloop) (should (equal (jupyter-ioloop-last-event ioloop) '(start))) (jupyter-ioloop-stop ioloop))) @@ -994,13 +994,13 @@ (setq jupyter-ioloop-test-handler-called nil) (jupyter-ioloop-add-callback ioloop `(lambda () (zmq-prin1 (list 'test "message")))) - (jupyter-ioloop-start ioloop :test) + (jupyter-test-ioloop-start ioloop) (jupyter-ioloop-stop ioloop) (should jupyter-ioloop-test-handler-called))) (ert-info ("Callback added after starting the ioloop") (let ((ioloop (jupyter-ioloop))) (setq jupyter-ioloop-test-handler-called nil) - (jupyter-ioloop-start ioloop :test) + (jupyter-test-ioloop-start ioloop) (should (process-live-p (oref ioloop process))) (jupyter-ioloop-add-callback ioloop `(lambda () (zmq-prin1 (list 'test "message")))) @@ -1014,7 +1014,7 @@ (setq jupyter-ioloop-test-handler-called nil) (jupyter-ioloop-add-setup ioloop (zmq-prin1 (list 'test "message"))) - (jupyter-ioloop-start ioloop :test) + (jupyter-test-ioloop-start ioloop) (jupyter-ioloop-stop ioloop) (should jupyter-ioloop-test-handler-called))) @@ -1024,7 +1024,7 @@ (setq jupyter-ioloop-test-handler-called nil) (jupyter-ioloop-add-teardown ioloop (zmq-prin1 (list 'test "message"))) - (jupyter-ioloop-start ioloop :test) + (jupyter-test-ioloop-start ioloop) (jupyter-ioloop-stop ioloop) (should jupyter-ioloop-test-handler-called))) @@ -1035,7 +1035,7 @@ (jupyter-ioloop-add-event ioloop test (data) "Echo DATA back to the parent process." (list 'test data)) - (jupyter-ioloop-start ioloop :test) + (jupyter-test-ioloop-start ioloop) (jupyter-send ioloop 'test "message") (jupyter-ioloop-stop ioloop) (should jupyter-ioloop-test-handler-called)))