Re: Public API change: How to handle function signature change gracefully

[Benjamin Andresen]

Thanks John and Nicolas.

I sort of arrived at the same point with my over-engineered approach and change 
of public API.

modified   lisp/org.el
@@ -5081,10 +5081,18 @@ This includes angle, plain, and bracket links."
(link (org-element-property :raw-link link-object))
(type (org-element-property :type link-object))
(path (org-element-property :path link-object))
+                (cbeg (org-element-property :contents-begin link-object))
+                (cend (org-element-property :contents-end link-object))
+                (contents (if (and cbeg cend)
+                              (buffer-substring-no-properties cbeg cend)))
(properties             ;for link's visible part
  (list
   'face (pcase (org-link-get-parameter type :face)
-                          ((and (pred functionp) face) (funcall face path))
+                          ((and (pred functionp) face) (let* ((number-of-args 
(cdr (func-arity face)))
+                                                              (kws (plist-put 
'() :contents contents)))
+                                                         (if (equal 
number-of-args 1)
+                                                             (funcall face 
path)
+                                                           (funcall face path 
kws))))
   ((and (pred facep) face) face)
   ((and (pred consp) face) face) ;anonymous
   (_ 'org-link))

This in turn resulted in more complexity on the callee side as I had to resolve 
the keywords.

So thanks for this simple fix I get rid of my over-engineered proposal: Having 
access to `org-element-context' is great for my usecase, because it means it 
will work on versions of Org that are already released.

Yay!

Best regards,
Benny


Apr 21, 2020, 22:07 by address@hidden:

> I think what Nicolas suggests is probably the easiest path. Here is one 
> example that does what I think you are looking for. I use a simple string 
> comparison on the contents, you could do something more sophisticated.
>
> #+BEGIN_SRC emacs-lisp
> (defun fruit-link-face (path)
>   (let* ((ln (org-element-context))
>  (start (org-element-property :contents-begin ln))
>  (end (org-element-property :contents-end ln))
>  (contents (if (and start end)
>        (buffer-substring start end)
>      nil)))
>     (if (and contents (stringp contents))
>  (if (string> contents "j")
>     '(:foreground "red")
>   '(:foreground "blue"))
>       'org-link)))
>
>
> (org-link-set-parameters "fruit"
>                          :face 'fruit-link-face)
> #+END_SRC
>
> #+RESULTS:
> | :face | fruit-link-face |
>
> [[fruit:mango ][test]].  # this will be red
>
>   fruit:apple. # regular org link.
>
>  [[fruit:apple][bera]]. # this will be blue
> John
>
> -----------------------------------
> Professor John Kitchin 
> Doherty Hall A207F
> Department of Chemical Engineering
> Carnegie Mellon University
> Pittsburgh, PA 15213
> 412-268-7803
> @johnkitchin
> http://kitchingroup.cheme.cmu.edu
>
>
> On Tue, Apr 21, 2020 at 1:57 PM Nicolas Goaziou <> address@hidden> > wrote:
>
>> Hello,
>>  
>>  Benjamin Andresen writes:
>>  >
>>  > I would like to change the public API of the :face part of 
>> `org-link-set-parameters':
>>  >
>>  >         (org-link-set-parameters "file" :face 'org-link)
>>  >
>>  > My ultimate goal is to have org-links be able to be have their face 
>> changed based on the contents, not just the path of the link.
>>  >
>>  > I found the relevant code in org.el in the function `org-activate-links':
>>  >
>>  >    'face (pcase (org-link-get-parameter type :face)
>>  >    ((and (pred functionp) face) (funcall face path))
>>  >    ((and (pred facep) face) face)
>>  >    ((and (pred consp) face) face) ;anonymous
>>  >    (_ 'org-link))
>>  >
>>  > and would like to change this to 
>>  >
>>  >    'face (pcase (org-link-get-parameter type :face)
>>  >    ((and (pred functionp) face) (funcall face path contents)) ;; this is 
>> the change
>>  >    ((and (pred facep) face) face)
>>  >    ((and (pred consp) face) face) ;anonymous
>>  >    (_ 'org-link))
>>  >
>>  > Now that will introduce a host of call issues because the callees don't 
>> expect to the amount of arguments changed under their bottom.
>>  >
>>  > I would like some guidance how I could get what I think is neat: The
>>  > contents of the bracket-style links as an additional parameter to set
>>  > faces on and not breaking existing hookups.
>>  
>>  Isn't the function called with point on the link? You may just need to
>>  extract the contents from the environment.
>>  
>>  Otherwise, a solution is to catch `wrong-number-of-arguments' error and
>>  call again the function with the old calling convention. See, e.g.,
>>  `org-link-open'.
>>  
>>  > To make this more palatable I would suggest it's changed so that the
>>  > 2nd argument will be a list of alists or keywords so to not have this
>>  > function signature problem if someone else comes up with a reason to
>>  > introduce yet more data.
>>  
>>  Beware the over-engineering. At this point, someone motivated enough can
>>  put an advice to the link fontification function.
>>  
>>  Regards,
>>  
>>  -- 
>>  Nicolas Goaziou
>>  
>>