Change `cider-interactive-eval-override` to make it more usable

[chpill]

Is your feature request related to a problem? Please describe.

The cider-interactive-eval-override variable can be used to change the way forms are evaluated by cider-interactive-eval, but contrary to what is said in the docstring, it does not actually forward cider-interactive-eval's last optional argument additional-params.

Consider the following example, where I want to be able to wrap every evaluation with a custom form which simply prints :wrapped before the target form is evaluated:

(defun println-override (form &optional callback bounds)
  (let ((cider-interactive-eval-override nil))
    (cider-interactive-eval (concat "(do (println :wrapped) " form ")")
                            callback
                            bounds)))

(setq cider-interactive-eval-override #'println-override)

This almost works, it correctly wraps any form evaluated and the println happens, but the pretty printing of some commands (like cider-pprint-eval-last-sexp) does not work anymore, because the last argument was not forwarded.

Describe the solution you'd like

Adding the missing argument to cider-interactive-eval-override's call site (funcall cider-interactive-eval-override form callback bounds additional-params) is very easy, but it would break backward compatibility. I'm not very fluent in Elisp so maybe there is a way to work around that.

With that change, the previous code can be changed to the following:

(defun println-override (form &optional callback bounds additional-params)
  (let ((cider-interactive-eval-override nil))
    (cider-interactive-eval (concat "(do (println :wrapped) " form ")")
                            callback
                            bounds
                            additional-params)))

(setq cider-interactive-eval-override #'println-override)

Which seems to work perfectly.

Additional context

As I searched for examples of correct usage of cider-interactive-eval-override on Github, it seems that it is in fact very rarely used. There might be a better way to wrap evaluation than this.

[vemv]

Thanks for the accurate report!

Taking a look at the commit that introduced this 02c4e80 - originally all params were forwarded.

Surely that was unawarely broken later.

I'm on the fence as to whether to introduce a breaking change.

Looks like we have a neat option: detect the arity of the user-set cider-interactive-eval-override, and only pass the last argument if cider-interactive-eval-override is ready for it.

Demo:

ielm> (help-function-arglist (lambda (a b c)))
(a b c)

ielm> (help-function-arglist 'mapcar)
(arg1 arg2)

Would you be interested in creating a PR?

Cheers - V

[chpill]

That's a great idea, I'll try to submit a PR that checks the number of arguments. Thanks for the pointer!

[chpill]

There is a func-arity that is even more convenient than help-function-arglist for our use case:

(let* ((arities (func-arity cider-interactive-eval-override))
       (min-arity (car arities))
       (max-arity (cdr arities)))
  (if (or (eq 'many max-arity)
          (and (numberp min-arity)
               (numberp max-arity)
               (<= min-arity 4 max-arity)))
    (funcall cider-interactive-eval-override form callback bounds additional-params)
    (funcall cider-interactive-eval-override form callback bounds)))

But there is an issue. When using apply-partially for example, the arglist becomes opaque, and from the outside, it'll look like the function takes any number of arguments (func-arity would return (0 . many)). I have found at least one example in the wild that would break because of this. I have thought of restricting functions using &rest arguments from accessing the new feature for the sake of backward compatibility, but then that would prevent other perfectly valid functions from working such as:

(defun println-override (form &rest extra-args)
  (let ((cider-interactive-eval-override nil))
    (apply 'cider-interactive-eval
           (concat "(do (println :wrapped) " form ")")
           extra-args)))

So maybe a better way is not to check the arities, but to try to call the arity-4 version anyway, and if an arity error occurs, fallback on the previous arity-3 call site:

(condition-case _
  (funcall cider-interactive-eval-override form callback bounds additional-params)
  (wrong-number-of-arguments
   (funcall cider-interactive-eval-override form callback bounds)))

@vemv what do you think?

[vemv]

Hi @chpill kudos for looking into it!

Yes, suporting a simple defun println-override (form &rest extra-args) would be important.

So your proposal seems great.

[chpill]

@vemv thanks a lot!