Feature request: advising + defadvice implementation

[jaidetree]

One aspect of Spacehammer that could stand to be improved is the ability to customize the implementation behavior of current or new features. For instance in an issue like #66 is going to require enhancing some core modal functions to change how bindings are created.

One option would be to design and architect some kind of hook or event system but then extra work will need to be done to update the system to use it, and it's a whole new API.

Another option would be to support more functions in the config.fnl. However, like the previous system we'll need to write checks for it in the relevant parts, additionally it muddles the purpose of config.fnl as it's now responsible for defining your configuration of spacehammer and the implementation.

My proposal would be to create a library similar to emacs-lisp advising functions, especially the doom emacs defadvice! macro.

An example from my dotconfig at https://github.com/eccentric-j/dotfiles/blob/c457fcba768f91a9fe2b7464221c97ba4dc3ad46/client/doom.d/config.el#L200-L206:

(defadvice! j/fix-js-multi-line-indent ()
  "Indent expression declarations by 2 just like the rest of the code"
  :after-while #'js--multi-line-declaration-indentation
  (let ((beg (match-beginning 0)))
    (when beg
      (goto-char beg)
      (+ js-indent-level (current-column)))))

Here I'm defining a function that fires after the js--multi-line-declaration-indentation function. Depending on the keyword used to represent how we're advising the function, I'll receive the original args sent to the function or the original function as the first argument.

Benefits

• You can define advice before a function is defined
• When inspecting the original function it will tell you that it's being advised and points to the new advising function
• If adapted to fennel, we could also localize our implementation customizations
• It's only marginally different from defining a function of the same name to replace it. This means less custom APIs to architect and maintain.
• We only have to swap (fn ...) with the name of the macro that registers the function with the advising state so (fn) becomes either (advisable-fn ...) or (defn ...) to add this to existing functionality.
• Provides the full power of fennel\lua at your disposal versus trying to build a custom API for specific behaviors in the system.

Tradeoffs

• There is a bit more overhead in the state and dispatching but we can measure it with the time macro to make sure it's not too slow. Based on previous performance tests, I doubt it will have a noticable impact beyond a fraction of a ms.
• Unlike emacs, system is enabled globally so all functions can be advised. In fennel will need to use a macro or pass a function to the register API to make it advisable.
• May not be the most common in fennel but may be something that is very easy to incorporate into the existing functionality to make it extendable.
• Could throw new users off if they are trying to debug when a function is called. However a.) New users likely won’t be using it early on b.) we will need to avoid using it in the default config other than registering advisable functions c.) We can enable a debug flag for logging the advising calls so it’s more clear.

API

• add-advice! function to advise a function
• remove-advice! function to remove an advising function
• defadvice! macro to define a function and call add-advice!.
• A advisable-fn or defn macro to register functions as advisable and create a handler to dispatch to advising functions.

[agzam]

I really like the idea. I can't think of a good use case for it at this point, but having it at our disposal perhaps would generate many good use-cases.

[jaidetree]

Some more use cases for this popped up:

It would help solve #86, #84, and #74. I think the time is right to prioritize this one.