proper use of add-function

proper use of add-function

[Eric Abrahamsen]

And another very basic question:

I can't get `add-function' to do its thing. I want to add a :filter-args
function to #'canonically-space-region, and because this is a minor
mode, I want it set locally. I've tried everything I can think of, but
either get no results (my advice function isn't called) or an error
saying that it wants a symbol, but I gave it a lambda. If I give it a
symbol, nothing happens.

(add-function
   :filter-args
   (local 'canonically-space-region)
   #'my-canonical-space-region)

(defun my-canonical-space-region (bounds)
  ;; etc
  (list (car bounds) (nth 1 bounds)))

What's wrong with this?

Thanks,
Eric

Re: proper use of add-function

[Noam Postavsky]

On 22 May 2018 at 18:58, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:

> (add-function
>    :filter-args
>    (local 'canonically-space-region)
>    #'my-canonical-space-region)

You're adding to the function value in the (non-existent) buffer-local
variable `canonically-space-region'

canonically-space-region’s value is
#f(advice-wrapper :filter-args #f(compiled-function
(&rest args)
#<bytecode 0x15054ad>)
my-canonical-space-region)
Local in buffer *scratch*; globally void

You probably want advice-add instead:

(advice-add 'canonically-space-region
        :filter-args
        #'my-canonical-space-region)

Note: this affects canonically-space-region globally, so you have to
change the implementation of my-canonical-space-region so that it's a
nop except when operating in the relevant buffers.

Re: proper use of add-function

[Michael Heerdegen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> And another very basic question:
>
> I can't get `add-function' to do its thing. I want to add a :filter-args
> function to #'canonically-space-region, and because this is a minor
> mode, I want it set locally.

I guess there is a problem with what you want to achieve: we have no
buffer local function bindings of symbols.

> (add-function
>    :filter-args
>    (local 'canonically-space-region)
>    #'my-canonical-space-region)
>
> (defun my-canonical-space-region (bounds)
>   ;; etc
>   (list (car bounds) (nth 1 bounds)))

Technically ok, but this tries to bind the (value cell of the) symbol
canonically-space-region.

What I typically do in this (quite common) scenario is to install the
advice (globally) and check for the mode in the `current-buffer' in the
advice.  I guess there is no alternative unless you find a different way
to reach what you want (like advising `canonical-space-region-function',
which doesn't exist).


Michael.

Re: proper use of add-function

[Eric Abrahamsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> And another very basic question:
>>
>> I can't get `add-function' to do its thing. I want to add a :filter-args
>> function to #'canonically-space-region, and because this is a minor
>> mode, I want it set locally.
>
> I guess there is a problem with what you want to achieve: we have no
> buffer local function bindings of symbols.
>
>> (add-function
>>    :filter-args
>>    (local 'canonically-space-region)
>>    #'my-canonical-space-region)
>>
>> (defun my-canonical-space-region (bounds)
>>   ;; etc
>>   (list (car bounds) (nth 1 bounds)))
>
> Technically ok, but this tries to bind the (value cell of the) symbol
> canonically-space-region.
>
> What I typically do in this (quite common) scenario is to install the
> advice (globally) and check for the mode in the `current-buffer' in the
> advice.  I guess there is no alternative unless you find a different way
> to reach what you want (like advising `canonical-space-region-function',
> which doesn't exist).

Okay, I see what you and Noam are saying, and in fact what the docstring
is saying. It's just pretty weird that `add-function' works on
variables, and `advice-add' works on functions. It's counterintuitive,
and I wonder if the docs couldn't make that more explicit. It's also a
bummer that my minor mode clobbers things globally, but I guess there's
no great harm done.

Thanks to both of you for being on call!

Eric

Re: proper use of add-function

[Michael Heerdegen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Okay, I see what you and Noam are saying, and in fact what the docstring
> is saying. It's just pretty weird that `add-function' works on
> variables, and `advice-add' works on functions.

Actually `add-function' works for "places" (including
`symbol-function'), so it's the more general and more low-level tool.
`advice-add' is higher-level and specialized on function names.

> It's counterintuitive, and I wonder if the docs couldn't make that
> more explicit.

I always found the doc ok (and explicit), but maybe the names are a bit
arbitrary.


Michael.

Re: proper use of add-function

[Eric Abrahamsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Okay, I see what you and Noam are saying, and in fact what the docstring
>> is saying. It's just pretty weird that `add-function' works on
>> variables, and `advice-add' works on functions.
>
> Actually `add-function' works for "places" (including
> `symbol-function'), so it's the more general and more low-level tool.
> `advice-add' is higher-level and specialized on function names.

I guess that's why I kept trying to make this work -- I thought the
`symbol-function' place would allow me to apply my advice to
'canonically-space-region. Why doesn't that work?

Re: proper use of add-function

[Noam Postavsky]

On 22 May 2018 at 20:09, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> Okay, I see what you and Noam are saying, and in fact what the docstring
>>> is saying. It's just pretty weird that `add-function' works on
>>> variables, and `advice-add' works on functions.

Yeah, this isn't the first time I've seen confusion over this (e.g.,
Bug#30241). The docstring is clear enough when you already know what
it says, but I've just added an extra note that should help guide
people toward advice-add.

[1: e3f00f5637]: 2018-05-22 20:08:01 -0400
  Clarify when to use advice-add vs add-function
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e3f00f5637a2790923a9c4c1d4b7dbf65027d8ce

>> Actually `add-function' works for "places" (including
>> `symbol-function'), so it's the more general and more low-level tool.
>> `advice-add' is higher-level and specialized on function names.
>
> I guess that's why I kept trying to make this work -- I thought the
> `symbol-function' place would allow me to apply my advice to
> 'canonically-space-region. Why doesn't that work?

(add-function
   :filter-args
   (symbol-function 'canonically-space-region)
   #'my-canonical-space-region)

This seems to work for me (although advice-add is preferable for
reasons listed in the manual).

Re: proper use of add-function

[Eric Abrahamsen]

On 05/22/18 20:19 PM, Noam Postavsky wrote:
> On 22 May 2018 at 20:09, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>> Michael Heerdegen <michael_heerdegen@web.de> writes:
>>
>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>
>>>> Okay, I see what you and Noam are saying, and in fact what the docstring
>>>> is saying. It's just pretty weird that `add-function' works on
>>>> variables, and `advice-add' works on functions.
>
> Yeah, this isn't the first time I've seen confusion over this (e.g.,
> Bug#30241). The docstring is clear enough when you already know what
> it says, but I've just added an extra note that should help guide
> people toward advice-add.
>
> [1: e3f00f5637]: 2018-05-22 20:08:01 -0400
>   Clarify when to use advice-add vs add-function
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e3f00f5637a2790923a9c4c1d4b7dbf65027d8ce

Thanks for doing that. That was definitely a "makes sense when you
already understand it" situation.

>>> Actually `add-function' works for "places" (including
>>> `symbol-function'), so it's the more general and more low-level tool.
>>> `advice-add' is higher-level and specialized on function names.
>>
>> I guess that's why I kept trying to make this work -- I thought the
>> `symbol-function' place would allow me to apply my advice to
>> 'canonically-space-region. Why doesn't that work?
>
> (add-function
>    :filter-args
>    (symbol-function 'canonically-space-region)
>    #'my-canonical-space-region)
>
> This seems to work for me (although advice-add is preferable for
> reasons listed in the manual).

Right, I get that now. I was only stuck on `add-function' because I was
under the impression that it would get me buffer-local behavior, which I
understand now it won't.

Thanks,
Eric