search

astahlman / ob-async

Asynchronous src_block execution for org-babel
343 stars 32 forks source link

ob-async: update function signature of ob-async-org-babel-execute-src… #93

Open stsquad opened 1 year ago

stsquad commented 1 year ago

…-block

Since 0625651e (Update to Org 9.6-3-ga4d38e) ob-async has been broken on the emacs-29 branch. Fix this by expanding the function to include the extra parameters.

This works but I'm unsure what effect it will have on older emacsen.

Fixes: #92 Signed-off-by: Alex Bennée alex.bennee@linaro.org

stsquad commented 1 year ago

I tested this change on 28.2 and it seemed to work ok.

yantar92 commented 1 year ago

Alex Bennée @.***> writes:

Since 0625651e (Update to Org 9.6-3-ga4d38e) ob-async has been broken on the emacs-29 branch. Fix this by expanding the function to include the extra parameters.

This works but I'm unsure what effect it will have on older emacsen.

When advising functions with optional parameters, I suggest to use &rest in the advise arglist + apply when calling the origin function. This is forward-compatible against adding new optional parameters.

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

stsquad commented 1 year ago

@yantar92 so something like this:

(defun ob-async-org-babel-execute-src-block ( &optional orig-fun arg info params
                                              &rest executor-type) ; since 9.6-3

are you suggesting replacing the direct funcalls:

    (funcall orig-fun arg info params executor-type))

with:

    (apply orig-fun arg info params executor-type))
yantar92 commented 1 year ago

Alex Bennée @.***> writes:

@yantar92 so something like this:

(defun ob-async-org-babel-execute-src-block ( &optional orig-fun arg info params
                                              &rest executor-type) ; since 9.6-3

Almost.

Just ... &rest other-args

It will work both in old and new versions and also if Org introduces new optional arguments.

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

stsquad commented 1 year ago

@yantar92 ok final update, and I tweaked the commit message as well. @astahlman happy to merge?