Suggestion: Use advice system for patching functions

[haji-ali]

When patching a function, I think it might be better (from an Emacs compatibility point of view) to define an advice with :override instead of over-defining the function.

I realize this would only work with functions, and not with other types (?), but since functions are the most patched type this might be worth it.

At the very least, something like this could be suitable

(defmacro el-patch-function (func-name &rest args)
  "Patches function FUNC-NAME.
Similar to `el-patch-defun' except the patch is applied as an
`:override' advice."
  (let* ((patch-name (make-symbol
                      (concat (symbol-name func-name) "@el-patch")))
         (new-args (cl-list* 'defun `(el-patch-swap ,func-name ,patch-name)
                             args)))
    `(progn
       (el-patch--definition ,new-args)
       (advice-add (quote ,func-name) :override (quote ,patch-name))
       ',patch-name)))

(and a similarly defined el-patch-define-function-template).

[raxod502]

We would still have to keep the old system for patching other definition types, so I'm not sure this would be a simplification. It also wouldn't work for the lazy-loading use case. Is there a particular limitation to the current implementation which you think would be alleviated by using advice instead?

[haji-ali]

There’s no limitation to the current implementation that I can think of. It’s more about using the standard Emacs way of patching (and unpatching), in the spirit of writing packages that prefer built-in features (like selectrum vs ivy).

[raxod502]

In that case, I'm not sure it would be a good idea to make the change, since it would result in significant loss of functionality.