* Public API change: How to handle function signature change gracefully
@ 2020-04-19 6:42 Benjamin Andresen via General discussions about Org-mode.
2020-04-21 17:57 ` Nicolas Goaziou
0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Andresen via General discussions about Org-mode. @ 2020-04-19 6:42 UTC (permalink / raw)
To: emacs-orgmode
Hello everyone,
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.
I'm thinking to change the above code to look at the callee's function signature and checking how many arguments it accepts and then use the right funcall for the situation.
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.
Thanks in advance,
Benjamin Andresen
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Public API change: How to handle function signature change gracefully
2020-04-19 6:42 Public API change: How to handle function signature change gracefully Benjamin Andresen via General discussions about Org-mode.
@ 2020-04-21 17:57 ` Nicolas Goaziou
2020-04-21 20:07 ` John Kitchin
[not found] ` <CAJ51ETrUxoWoraMxKTCRnzNxtmj5kwVWBAWvAeL1PSZvJs5hjw@mail.gmail.com-M5TLt9R----2>
0 siblings, 2 replies; 4+ messages in thread
From: Nicolas Goaziou @ 2020-04-21 17:57 UTC (permalink / raw)
To: Benjamin Andresen via General discussions about Org-mode.
Cc: Benjamin Andresen
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Public API change: How to handle function signature change gracefully
2020-04-21 17:57 ` Nicolas Goaziou
@ 2020-04-21 20:07 ` John Kitchin
[not found] ` <CAJ51ETrUxoWoraMxKTCRnzNxtmj5kwVWBAWvAeL1PSZvJs5hjw@mail.gmail.com-M5TLt9R----2>
1 sibling, 0 replies; 4+ messages in thread
From: John Kitchin @ 2020-04-21 20:07 UTC (permalink / raw)
To: Benjamin Andresen via General discussions about Org-mode.,
Benjamin Andresen
[-- Attachment #1: Type: text/plain, Size: 3184 bytes --]
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 <mail@nicolasgoaziou.fr>
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
>
>
[-- Attachment #2: Type: text/html, Size: 4286 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Public API change: How to handle function signature change gracefully
[not found] ` <CAJ51ETrUxoWoraMxKTCRnzNxtmj5kwVWBAWvAeL1PSZvJs5hjw@mail.gmail.com-M5TLt9R----2>
@ 2020-04-21 21:56 ` Benjamin Andresen via General discussions about Org-mode.
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Andresen via General discussions about Org-mode. @ 2020-04-21 21:56 UTC (permalink / raw)
To: Emacs Orgmode
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 jkitchin@andrew.cmu.edu:
> 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 <> mail@nicolasgoaziou.fr> > 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
>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-21 21:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-19 6:42 Public API change: How to handle function signature change gracefully Benjamin Andresen via General discussions about Org-mode.
2020-04-21 17:57 ` Nicolas Goaziou
2020-04-21 20:07 ` John Kitchin
[not found] ` <CAJ51ETrUxoWoraMxKTCRnzNxtmj5kwVWBAWvAeL1PSZvJs5hjw@mail.gmail.com-M5TLt9R----2>
2020-04-21 21:56 ` Benjamin Andresen via General discussions about Org-mode.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).