emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* ol.el: add description format parameter to org-link-parameters
@ 2022-03-28 23:15 Hugo Heagren
  2022-03-28 23:15 ` [PATCH] " Hugo Heagren
  0 siblings, 1 reply; 8+ messages in thread
From: Hugo Heagren @ 2022-03-28 23:15 UTC (permalink / raw)
  To: emacs-orgmode

Suggested patch to add a new link parameter controlling how the
default description is generated. As explained in the commit, this
behaviour is often similar between links of the same type, but differs
between those types, so a parameter seems like a good place to specify
it.

I've tried to make `org-insert-link' behave sensibly with regard to
all the possible options -- hope it's alright.

Blue skies, Hugo




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] ol.el: add description format parameter to org-link-parameters
  2022-03-28 23:15 ol.el: add description format parameter to org-link-parameters Hugo Heagren
@ 2022-03-28 23:15 ` Hugo Heagren
  2022-04-04  9:49   ` Ihor Radchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Hugo Heagren @ 2022-03-28 23:15 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Hugo Heagren

* ol.el (org-link-parameters): add parameter `:default-description', a
string or a function.
* (org-insert-link): if no description is provided (pre-existing or as
an argument), next option is to use the `:default-description' (if
non-nil) parameter to generate one.

Default descriptions are predictable within a link type, but because
link types are quite diverse, are NOT predictable across many types. A
type-parameter is thus a good place to store information on the
default description.
---
 lisp/ol.el | 49 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index 1b2bb9a9a..af0aaaf35 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -140,6 +140,13 @@ link.
   Function that inserts a link with completion.  The function
   takes one optional prefix argument.
 
+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description. A string is used as-is, a function is
+  called with the full link text as the sole argument, and should
+  return a single string.
+
 `:display'
 
   Value for `invisible' text property on the hidden parts of the
@@ -1761,11 +1768,14 @@ prefix negates `org-link-keep-stored-after-insertion'.
 If the LINK-LOCATION parameter is non-nil, this value will be used as
 the link location instead of reading one interactively.
 
-If the DESCRIPTION parameter is non-nil, this value will be used as the
-default description.  Otherwise, if `org-link-make-description-function'
-is non-nil, this function will be called with the link target, and the
-result will be the default link description.  When called non-interactively,
-don't allow to edit the default description."
+If the DESCRIPTION parameter is non-nil, this value will be used
+as the default description.  If not, and the chosen link type has
+a non-nil `:default-description' parameter, that is used to
+generate a description as described in `org-link-parameters'
+docstring. Otherwise, if `org-link-make-description-function' is
+non-nil, this function will be called with the link target, and
+the result will be the default link description.  When called
+non-interactively, don't allow to edit the default description."
   (interactive "P")
   (let* ((wcf (current-window-configuration))
 	 (origbuf (current-buffer))
@@ -1775,7 +1785,7 @@ don't allow to edit the default description."
 	 (desc region)
 	 (link link-location)
 	 (abbrevs org-link-abbrev-alist-local)
-	 entry all-prefixes auto-desc)
+	 entry all-prefixes auto-desc type)
     (cond
      (link-location)		      ; specified by arg, just use it.
      ((org-in-regexp org-link-bracket-re 1)
@@ -1842,6 +1852,7 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 		      (and (equal ":" (substring link -1))
 			   (member (substring link 0 -1) all-prefixes)
 			   (setq link (substring link 0 -1))))
+              (setq type link)
 	      (setq link (with-current-buffer origbuf
 			   (org-link--try-special-completion link)))))
 	(set-window-configuration wcf)
@@ -1918,14 +1929,24 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
       (let ((initial-input
 	     (cond
 	      (description)
-	      ((not org-link-make-description-function) desc)
-	      (t (condition-case nil
-		     (funcall org-link-make-description-function link desc)
-		   (error
-		    (message "Can't get link description from %S"
-			     (symbol-name org-link-make-description-function))
-		    (sit-for 2)
-		    nil))))))
+              (desc)
+              ((org-link-get-parameter
+                type
+                :default-description)
+               (let ((def (org-link-get-parameter
+                           type
+                           :default-description)))
+                 (cond
+                  ((stringp def) def)
+                  ((functionp def)
+                   (funcall def link)))))
+	      (org-link-make-description-function
+               (funcall org-link-make-description-function link desc))
+              (t (error
+		  (message "Can't get link description from %S"
+			   (symbol-name org-link-make-description-function))
+		  (sit-for 2)
+		  nil)))))
 	(setq desc (if (called-interactively-p 'any)
 		       (read-string "Description: " initial-input)
 		     initial-input))))
-- 
2.20.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ol.el: add description format parameter to org-link-parameters
  2022-03-28 23:15 ` [PATCH] " Hugo Heagren
@ 2022-04-04  9:49   ` Ihor Radchenko
  2022-04-05 19:29     ` [PATCH v2] " Hugo Heagren
  0 siblings, 1 reply; 8+ messages in thread
From: Ihor Radchenko @ 2022-04-04  9:49 UTC (permalink / raw)
  To: Hugo Heagren; +Cc: emacs-orgmode

Hugo Heagren <hugo@heagren.com> writes:

> * ol.el (org-link-parameters): add parameter `:default-description', a
> string or a function.

LGTM in general. Note that I proposed (and never followed up on) a
similar patch in the past:
https://orgmode.org/list/87pnauvxht.fsf@localhost

I like that you introduced an option to set default description to
string, but I have several comments on the patch itself.


> -	      (t (condition-case nil
> -		     (funcall org-link-make-description-function link desc)
> +	      (org-link-make-description-function
> +               (funcall org-link-make-description-function link desc))
> +              (t (error
> +		  (message "Can't get link description from %S"
> +			   (symbol-name org-link-make-description-function))
> +		  (sit-for 2)
> +		  nil)))))

It looks like you removed condition-case used to catch errors on
org-link-make-description function. I am pretty sure that it is not
intentional.

Similar condition-case could also be used for the :default-description
function.

+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description. A string is used as-is, a function is
+  called with the full link text as the sole argument, and should
+  return a single string.

Why not two arguments like in org-link-make-description-function?

Best,
Ihor



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ol.el: add description format parameter to org-link-parameters
  2022-04-04  9:49   ` Ihor Radchenko
@ 2022-04-05 19:29     ` Hugo Heagren
  2022-04-07  5:13       ` Ihor Radchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Hugo Heagren @ 2022-04-05 19:29 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Hugo Heagren

* ol.el (org-link-parameters): add parameter `:default-description', a
string or a function.
* (org-insert-link): if no description is provided (pre-existing or as
an argument), next option is to use the `:default-description' (if
non-nil) parameter to generate one.

Default descriptions are predictable within a link type, but because
link types are quite diverse, are NOT predictable across many types. A
type-parameter is thus a good place to store information on the
default description.
---

I've added the condition-case back to the check on
`org-link-make-description', and added a new one to the check for the
`:default-description' parameter, as Ihor suggested. I've also
modified the handling of that parameter, to reflect
`org-link-make-description', and updated the docstring accordingly.

Apologies if the subject formatting is not correct, I'm still getting
the hang of git-send-email.

Hugo

 lisp/ol.el | 43 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index 1b2bb9a9a..e74ef8dda 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -140,6 +140,15 @@ link.
   Function that inserts a link with completion.  The function
   takes one optional prefix argument.
 
+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description. A string is used as-is, a function is
+  called with two arguments: the full link text, and the
+  description generated by `org-insert-linke'. It should return
+  the description to use (this reflects the behaviour of
+  `org-link-make-description-function').
+
 `:display'
 
   Value for `invisible' text property on the hidden parts of the
@@ -1761,11 +1770,14 @@ prefix negates `org-link-keep-stored-after-insertion'.
 If the LINK-LOCATION parameter is non-nil, this value will be used as
 the link location instead of reading one interactively.
 
-If the DESCRIPTION parameter is non-nil, this value will be used as the
-default description.  Otherwise, if `org-link-make-description-function'
-is non-nil, this function will be called with the link target, and the
-result will be the default link description.  When called non-interactively,
-don't allow to edit the default description."
+If the DESCRIPTION parameter is non-nil, this value will be used
+as the default description.  If not, and the chosen link type has
+a non-nil `:default-description' parameter, that is used to
+generate a description as described in `org-link-parameters'
+docstring. Otherwise, if `org-link-make-description-function' is
+non-nil, this function will be called with the link target, and
+the result will be the default link description.  When called
+non-interactively, don't allow to edit the default description."
   (interactive "P")
   (let* ((wcf (current-window-configuration))
 	 (origbuf (current-buffer))
@@ -1775,7 +1787,7 @@ don't allow to edit the default description."
 	 (desc region)
 	 (link link-location)
 	 (abbrevs org-link-abbrev-alist-local)
-	 entry all-prefixes auto-desc)
+	 entry all-prefixes auto-desc type)
     (cond
      (link-location)		      ; specified by arg, just use it.
      ((org-in-regexp org-link-bracket-re 1)
@@ -1842,6 +1854,7 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 		      (and (equal ":" (substring link -1))
 			   (member (substring link 0 -1) all-prefixes)
 			   (setq link (substring link 0 -1))))
+              (setq type link)
 	      (setq link (with-current-buffer origbuf
 			   (org-link--try-special-completion link)))))
 	(set-window-configuration wcf)
@@ -1918,7 +1931,23 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
       (let ((initial-input
 	     (cond
 	      (description)
-	      ((not org-link-make-description-function) desc)
+              (desc)
+              ((org-link-get-parameter
+                type
+                :default-description)
+               (condition-case nil
+                   (let ((def (org-link-get-parameter
+                               type
+                               :default-description)))
+                     (cond
+                      ((stringp def) def)
+                      ((functionp def)
+                       (funcall def link desc))))
+                 (error
+                  (message "Can't get link description from %S"
+			   (symbol-name def))
+		  (sit-for 2)
+                  nil)))
 	      (t (condition-case nil
 		     (funcall org-link-make-description-function link desc)
 		   (error
-- 
2.20.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] ol.el: add description format parameter to org-link-parameters
  2022-04-05 19:29     ` [PATCH v2] " Hugo Heagren
@ 2022-04-07  5:13       ` Ihor Radchenko
  2022-06-21 12:03         ` [PATCH v3] " Hugo Heagren
  0 siblings, 1 reply; 8+ messages in thread
From: Ihor Radchenko @ 2022-04-07  5:13 UTC (permalink / raw)
  To: Hugo Heagren; +Cc: emacs-orgmode

Hugo Heagren <hugo@heagren.com> writes:

> I've added the condition-case back to the check on
> `org-link-make-description', and added a new one to the check for the
> `:default-description' parameter, as Ihor suggested. I've also
> modified the handling of that parameter, to reflect
> `org-link-make-description', and updated the docstring accordingly.

Thanks!

Some more comments:

> +               (condition-case nil
> +                   (let ((def (org-link-get-parameter
> +                               type
> +                               :default-description)))
> +                     (cond
> +                      ((stringp def) def)
> +                      ((functionp def)
> +                       (funcall def link desc))))
> +                 (error
> +                  (message "Can't get link description from %S"
> +			   (symbol-name def))

Firstly, def will be undefined inside the error clause.

Secondly, when :default-description is a lambda expression and that
expression fails, your code will fail with "wrong-type-argument symbolp"
when executing (symbol-name def). Please consider cases when `def' is
not a string and not a function symbol.

I recommend writing tests for org-insert-link in testing/lisp/test-ol.el
and testing expected behaviour for various values of
:default-description.

> Apologies if the subject formatting is not correct, I'm still getting
> the hang of git-send-email.

You don't have to use git-send-email to submit simple patches. An easier
(as for me) alternative is a simple email with attached patch. magit
makes it trivial to create patch files.

Best,
Ihor


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] ol.el: add description format parameter to org-link-parameters
  2022-04-07  5:13       ` Ihor Radchenko
@ 2022-06-21 12:03         ` Hugo Heagren
  2022-06-21 13:41           ` Robert Pluim
  2022-06-21 15:01           ` Max Nikulin
  0 siblings, 2 replies; 8+ messages in thread
From: Hugo Heagren @ 2022-06-21 12:03 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 1594 bytes --]

I've finally had time to the `:default-description' parameter in
`org-link-parameters'.

Ihor Radchenko <yantar92@gmail.com> writes:

> I recommend writing tests for org-insert-link in
> testing/lisp/test-ol.el and testing expected behaviour for various
> values of :default-description.

Good advice! I haven't written many tests before, so this was a
challenge. I've attached a patch with some tests for the expected
behaviour. These include a couple of macros and a function, to stop
the definitions being too huge and unwieldy.

All the tests pass on my machine.

They're very nearly just right, but for some reason
`test-ol-with-link-parameters-as' doesn't always reset the parameters
correctly for me. However I have them set to originally, it sets
`:default-description' to a lambda. This might be a quirk at my end
though -- if someone else could have a look I'd be very grateful.

The tests showed that my previous approach doesn't work generally
enough. I've moved where `type' is defined in the code, to cover all
cases.

> Firstly, def will be undefined inside the error clause.

I've defined def a bit further up in a `let', and run the error clause
inside this.

> Secondly, when :default-description is a lambda expression and that
> expression fails, your code will fail with "wrong-type-argument
> symbolp" when executing (symbol-name def). Please consider cases
> when `def' is not a string and not a function symbol.

Dealt with this by just using `message "<text> %S: "', since %S will
print a symbol, a string or a lambda correctly.

Hope this is a bit better!

Best,
Hugo

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-test-ol-tests-for-default-description-param-when-ins.patch --]
[-- Type: text/x-diff, Size: 3476 bytes --]

From 5a44edfda4e09a95bba1327c2758b2637e5b16bd Mon Sep 17 00:00:00 2001
From: Hugo Heagren <hugo@heagren.com>
Date: Tue, 21 Jun 2022 12:45:50 +0100
Subject: [PATCH 2/2] test-ol: tests for default-description param when
 inserting links

Add tests for various values of `:default-description' in
`org-link-parameters'.
---
 testing/lisp/test-ol.el | 77 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el
index 429bb52ee..7524d73af 100644
--- a/testing/lisp/test-ol.el
+++ b/testing/lisp/test-ol.el
@@ -625,5 +625,82 @@ See https://github.com/yantar92/org/issues/4."
     (test-ol-parse-link-in-text
         "The <point>http://foo.com/(something)?after=parens link"))))
 
+;;; Insert Links
+
+(defmacro test-ol-with-link-parameters-as (type parameters &rest body)
+  "Pass TYPE/PARAMETERS to `org-link-parameters' and execute BODY.
+
+Save the original value of `org-link-parameters', execute
+`org-link-set-parameters' with the relevant args, execute BODY
+and restore `org-link-parameters'.
+
+TYPE is as in `org-link-set-parameters'.  PARAMETERS is a plist to
+be passed to `org-link-set-parameters'."
+  `(progn
+     (setq orig-parameters org-link-parameters)
+     (org-link-set-parameters ,type ,@parameters)
+     (let ((rtn (progn ,@body)))
+       (setq org-link-parameters orig-parameters)
+       rtn)))
+
+(defun test-ol-insert-link-get-desc (&optional link-location description)
+  "Insert link in temp buffer, return description.
+
+LINK-LOCATION and DESCRIPTION are passed to
+`org-insert-link' (COMPLETE-FILE is always nil)."
+  (org-test-with-temp-text ""
+    (org-insert-link nil link-location description)
+    (save-match-data
+      (when (and
+             (org-in-regexp org-link-bracket-re 1)
+             (match-end 2))
+        (match-string-no-properties 2)))))
+
+(defun test-ol/return-foobar (_link-test _desc)
+  "Return string \"foobar\".
+
+Take (and ignore) arguments conforming to `:default-description'
+API in `org-link-parameters'.  Used in test
+`test-ol/insert-link-default-description', for the case where
+`:default-description' is a function symbol."
+  "foobar")
+
+(ert-deftest test-ol/insert-link-default-description ()
+  "Test `:default-description' parameter handling."
+  ;; String case
+  (should
+   (string=
+    "foobar"
+    (test-ol-with-link-parameters-as
+     "id" (:default-description "foobar")
+     (test-ol-insert-link-get-desc "id:foo-bar"))))
+  ;; Lambda case
+  (should
+   (string=
+    "foobar"
+    (test-ol-with-link-parameters-as
+     "id" (:default-description (lambda (_link-test _desc) "foobar"))
+     (test-ol-insert-link-get-desc "id:foo-bar"))))
+  ;; Function symbol case
+  (should
+   (string=
+    "foobar"
+    (test-ol-with-link-parameters-as
+     "id" (:default-description #'test-ol/return-foobar)
+     (test-ol-insert-link-get-desc "id:foo-bar"))))
+  ;; `:default-description' parameter is defined, but doesn't return a
+  ;; string
+  (should-error
+   (test-ol-with-link-parameters-as
+    "id" (:default-description #'ignore)
+    (test-ol-insert-link-get-desc "id:foo-bar")))
+  ;; Description argument should override `:default-description'
+  (should
+   (string=
+    "notfoobar"
+    (test-ol-with-link-parameters-as
+     "id" (:default-description "foobar")
+     (test-ol-insert-link-get-desc "id:foo-bar" "notfoobar")))))
+
 (provide 'test-ol)
 ;;; test-ol.el ends here
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-ol.el-add-description-format-parameter-to-org-link-p.patch --]
[-- Type: text/x-diff, Size: 4870 bytes --]

From 437155c2394b5c9961ecc0af4e1e1a54b63416fe Mon Sep 17 00:00:00 2001
From: Hugo Heagren <hugo@heagren.com>
Date: Mon, 28 Mar 2022 23:18:45 +0100
Subject: [PATCH 1/2] ol.el: add description format parameter to
 org-link-parameters

* ol.el (org-link-parameters): add parameter `:default-description', a
string or a function.
* (org-insert-link): if no description is provided (pre-existing or as
an argument), next option is to use the `:default-description' (if
non-nil) parameter to generate one.

Default descriptions are predictable within a link type, but because
link types are quite diverse, are NOT predictable across many types. A
type-parameter is thus a good place to store information on the
default description.
---
 lisp/ol.el | 47 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index d4bd0e40c..1ab3a5f76 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -141,6 +141,15 @@ link.
   Function that inserts a link with completion.  The function
   takes one optional prefix argument.
 
+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description.  A string is used as-is, a function is
+  called with two arguments: the full link text, and the
+  description generated by `org-insert-link'.  It should return
+  the description to use (this reflects the behaviour of
+  `org-link-make-description-function').
+
 `:display'
 
   Value for `invisible' text property on the hidden parts of the
@@ -1804,11 +1813,14 @@ prefix negates `org-link-keep-stored-after-insertion'.
 If the LINK-LOCATION parameter is non-nil, this value will be used as
 the link location instead of reading one interactively.
 
-If the DESCRIPTION parameter is non-nil, this value will be used as the
-default description.  Otherwise, if `org-link-make-description-function'
-is non-nil, this function will be called with the link target, and the
-result will be the default link description.  When called non-interactively,
-don't allow to edit the default description."
+If the DESCRIPTION parameter is non-nil, this value will be used
+as the default description.  If not, and the chosen link type has
+a non-nil `:default-description' parameter, that is used to
+generate a description as described in `org-link-parameters'
+docstring. Otherwise, if `org-link-make-description-function' is
+non-nil, this function will be called with the link target, and
+the result will be the default link description.  When called
+non-interactively, don't allow to edit the default description."
   (interactive "P")
   (let* ((wcf (current-window-configuration))
 	 (origbuf (current-buffer))
@@ -1818,7 +1830,7 @@ don't allow to edit the default description."
 	 (desc region)
 	 (link link-location)
 	 (abbrevs org-link-abbrev-alist-local)
-	 entry all-prefixes auto-desc)
+	 entry all-prefixes auto-desc type)
     (cond
      (link-location)		      ; specified by arg, just use it.
      ((org-in-regexp org-link-bracket-re 1)
@@ -1893,6 +1905,13 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
       (or entry (push link org-link--insert-history))
       (setq desc (or desc (nth 1 entry)))))
 
+    (setq type
+          (cond
+           ((string-match (rx (: string-start (submatch (eval `(or ,@all-prefixes))) ":")) link)
+            (match-string 1 link))
+           ((file-name-absolute-p link) "file")
+           ((string-match "\\`\\.\\.?/" link) "file")))
+
     (when (funcall (if (equal complete-file '(64)) 'not 'identity)
 		   (not org-link-keep-stored-after-insertion))
       (setq org-stored-links (delq (assoc link org-stored-links)
@@ -1961,12 +1980,24 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
       (let ((initial-input
 	     (cond
 	      (description)
-	      ((not org-link-make-description-function) desc)
+              (desc)
+              ((org-link-get-parameter type :default-description)
+               (let ((def (org-link-get-parameter type :default-description)))
+                 (condition-case nil
+                     (cond
+                      ((stringp def) def)
+                      ((functionp def)
+                       (funcall def link desc)))
+                   (error
+                    (message "Can't get link description from org link parameter `:default-description': %S"
+			     def)
+		    (sit-for 2)
+                    nil))))
 	      (t (condition-case nil
 		     (funcall org-link-make-description-function link desc)
 		   (error
 		    (message "Can't get link description from %S"
-			     (symbol-name org-link-make-description-function))
+		             org-link-make-description-function)
 		    (sit-for 2)
 		    nil))))))
 	(setq desc (if (called-interactively-p 'any)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] ol.el: add description format parameter to org-link-parameters
  2022-06-21 12:03         ` [PATCH v3] " Hugo Heagren
@ 2022-06-21 13:41           ` Robert Pluim
  2022-06-21 15:01           ` Max Nikulin
  1 sibling, 0 replies; 8+ messages in thread
From: Robert Pluim @ 2022-06-21 13:41 UTC (permalink / raw)
  To: Hugo Heagren; +Cc: emacs-orgmode

>>>>> On Tue, 21 Jun 2022 13:03:32 +0100, Hugo Heagren <hugo@heagren.com> said:

    Hugo> They're very nearly just right, but for some reason
    Hugo> `test-ol-with-link-parameters-as' doesn't always reset the parameters
    Hugo> correctly for me. However I have them set to originally, it sets
    Hugo> `:default-description' to a lambda. This might be a quirk at my end
    Hugo> though -- if someone else could have a look I'd be very grateful.

They way your macro is defined, it uses `orig-parameters' in a way
that it will leak into the global namespace. Better would be to do

(let ((orig-parameters org-link-parameters))
       ...
      (setq org-link-parameters orig-parameters)

Robert
-- 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] ol.el: add description format parameter to org-link-parameters
  2022-06-21 12:03         ` [PATCH v3] " Hugo Heagren
  2022-06-21 13:41           ` Robert Pluim
@ 2022-06-21 15:01           ` Max Nikulin
  1 sibling, 0 replies; 8+ messages in thread
From: Max Nikulin @ 2022-06-21 15:01 UTC (permalink / raw)
  To: emacs-orgmode

On 21/06/2022 19:03, Hugo Heagren wrote:
> +(defmacro test-ol-with-link-parameters-as (type parameters &rest body)
[...]
> +  `(progn
> +     (setq orig-parameters org-link-parameters)

I can easily miss something, but wouldn't it be enough to use let-binding

`(let ((org-link-parameters org-link-parameters))
    ,@body)

Otherwise it is better to use something like `condition-case' to restore 
original state even when some error is signaled.

> +     (org-link-set-parameters ,type ,@parameters)
> +     (let ((rtn (progn ,@body)))
> +       (setq org-link-parameters orig-parameters)
> +       rtn)))




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-06-21 15:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-28 23:15 ol.el: add description format parameter to org-link-parameters Hugo Heagren
2022-03-28 23:15 ` [PATCH] " Hugo Heagren
2022-04-04  9:49   ` Ihor Radchenko
2022-04-05 19:29     ` [PATCH v2] " Hugo Heagren
2022-04-07  5:13       ` Ihor Radchenko
2022-06-21 12:03         ` [PATCH v3] " Hugo Heagren
2022-06-21 13:41           ` Robert Pluim
2022-06-21 15:01           ` Max Nikulin

Code repositories for project(s) associated with this 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).