Subject: Re: bug#66908: Exposing more public nadvice API

From: Philip Kaludercic <[email-masked]>
Date: Sat, 04 Nov 2023 09:58:47 +0000

Stefan Monnier <[email-masked]> writes:

>> Sure, in Philip's package do-at-point, a function defined to "act" on
>> the `thing' at point are given different arguments depending on the
>> minimum number of arguments required by the function:
>
> Ah :-(
>
> So a kind of "unimplementable semantics" for DWIM purposes.
>
>>         (pcase (car (func-arity func))
>>                     ^^^^^^^^^^^^^^^^^
>>           (0 (funcall func))
>>           (1 (funcall func (buffer-substring (car bound) (cdr bound))))
>>           (2 (funcall func (car bound) (cdr bound)))
>>           (_ (error "Unsupported signature: %S" func)))))
>
> I recommend:
>
>     (condition-case nil
>         (funcall func (car bound) (cdr bound))
>       (wrong-number-of-arguments
>        (condition-case nil
>            (funcall func (buffer-substring (car bound) (cdr bound)))
>          (wrong-number-of-arguments
>           (funcall func)))))
>
> :-)
>
> Works with advice and other wrappers without having to worry about
> `indirect-function`, autoloading, etc...

The main issue here is that this checks if a function accepts up to two
arguments, but we are interested in the minimal number of arguments.
I guess that turning this around should work, right:

     (condition-case nil
         (funcall func)
       (wrong-number-of-arguments
        (condition-case nil
            (funcall func (buffer-substring (car bound) (cdr bound)))
          (wrong-number-of-arguments
           (funcall func (car bound) (cdr bound))))))

?

>>>>     (func-arity (advice--cd*r (indirect-function 'xref-find-definitions))) ;; ⇒ (1 . 1)
>>>> which is the right return value.  It might be nice to not have to call
>>>> `indirect-function' here for the "global" function but you can be a
>>>> better judge of that.
>>> Don't know what you mean by "global" function.
>> By "global", I mean the new global function advice-cd*r or somesuch that
>> might eventually be added from this discussion.
>
> Ah, I see.  I don't think `advice-cd*r` should follow aliases in
> general.  But indeed, you may have an `advice` object whose
> `advice-cd*r` is a symbol, whose definition is another advice object,
> etc...
>
> Another good reason to prefer the `condition-case` approach :-)
>
>>> By "autoloaded" do you mean "setup to be loaded on demand but not yet
>>> loaded", or do you mean "had been setup to be loaded on demand and has
>>> been loaded already"?
>> The former obviously.
>
> In that case, `indirect-function` would not see the advice, then
> `func-arity` would cause the target to be (auto)loaded, during which
> a previously pending advice could be installed and it would return the
> dreaded (0 . many) from the advice it sees in the definition.
>
> Again, using `condition-case` side steps the issue.
>
>
>         Stefan