Timeline for Add-advice :before org-edit-special
Current License: CC BY-SA 4.0
12 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Apr 30, 2020 at 1:58 | comment | added | phils | I believe the argument list you specify replaces the argument list for the original function (at least so far as callers are concerned), and so yes -- unless you are intentionally (and carefully) making changes to it, it needs to match the original. It feels like an intentional design choice to have made that a required part of the new advice, and I also prefer the old system in this regard -- I wrote a lot of defadvice, and frequently manipulated argument values, but never once wanted to change the argument list. | |
| Apr 29, 2020 at 22:41 | comment | added | Adam | @phils Does the argument list always have to match the function you are advising? If so, it seems like defadvice is a more compact way of getting at the same thing... unless adding the argument list lends some additional flexibility? | |
| Apr 27, 2020 at 23:59 | comment | added | phils | This is why, when you set the argument list of org-edit-special to nil and something subsequently called that function with an argument, you ended up with an error "Wrong number of arguments". | |
| Apr 27, 2020 at 23:53 | comment | added | phils | To be clear: (&optional arg) is the argument list for org-edit-special, the function which you are advising. If you were advising something different, the argument list would likely be different. Advice can mess with the arguments, so if you change that list you're changing the behaviour. Specifying this was optional for defadvice but is required for define-advice. | |
| Apr 27, 2020 at 23:10 | comment | added | Adam | @phils What is the purpose of this new text (&optional arg)? Does this add some additional flexibility in the way that we advise functions that was not found in the defadvice pattern? | |
| Apr 27, 2020 at 22:15 | comment | added | phils | FWIW argument handling is an area where I personally think the old advice is much nicer than the new advice. | |
| Apr 27, 2020 at 22:10 | comment | added | phils | @Adam I just meant that if it was valid to use nil then, in those cases, you could alternatively use () because the two things are the same. My impression is that nadvice requires you to state the argument list, because it uses it -- so if it's not empty then you can't say that it is without breaking calls to the function. In the case of org-edit-special, per the code in the answer, the appropriate value is (&optional arg). | |
| Apr 27, 2020 at 20:59 | comment | added | Adam | @phils I just tested this out (define-advice org-edit-special (:before () my-big-advice) (message "bingo!")). After evaluating, with point on an R src block, I ran org-edit-special and received the following error: Wrong number of arguments: (lambda nil (message "bingo!")), 1. Any ideas? | |
| Apr 27, 2020 at 11:35 | comment | added | phils | Using nadvice.el tends to be strongly recommended nowadays. Instances of old advice in Emacs core and in ELPA tend to get re-written. It's probably best to get familiar with nadvice. Using the define-advice macro is the easiest way to port old advice for the most part, and personally I think it's the most readable of the options provided by the nadvice library (certainly if you're familiar with the old advice library); so if it suits your purpose then I would suggest using it. Entirely up to you, though. | |
| Apr 27, 2020 at 11:22 | comment | added | phils | I'll just add that as nil and () are the same thing in elisp, you could indeed use (). | |
| Apr 27, 2020 at 8:05 | comment | added | Adam | Thanks @phils - I find this topic extremely confusing, in particular because there appears to be no consensus in the documentation on how to get this done. Can you speak to the advantages/disadvantages of each approach? What is the benefit of using nadvice.el versus the one I outlined above? And how about the approach that NickD mentioned, using advice-add? Also, in your code, if the optional args are not needed, do you still include the parentheses? I.e., (define-advice org-edit-special (:before () my-big-advice) (message “bingo!”)) | |
| Apr 27, 2020 at 6:15 | history | answered | phils | CC BY-SA 4.0 |