From: Hugo Heagren <hugo@heagren.com>
To: Ihor Radchenko <yantar92@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH v5] ol.el: add description format parameter to org-link-parameters
Date: Sat, 16 Jul 2022 22:20:42 +0100 [thread overview]
Message-ID: <87tu7gkb4l.fsf@heagren.com> (raw)
In-Reply-To: 87cze5e84m.fsf@localhost
[-- Attachment #1: Type: text/plain, Size: 3094 bytes --]
> Can you also update the documentation for
> org-link-make-description-function?
I'm not sure what sort of documentation you have in mind? What should
I add?
> I have realized that even better option would be using map-do.
Agreed. Should be in the current version.
With regard to Max's comments:
> I am in doubts concerning "default-description" as the parameter
> name. I would consider either more specific "insert-description"
Having read your thoughts, I agree. I've converted the patch to use
"insert-description" where-ever relevant.
> Have I missed something or the whole macro may be simplified to just
> copy `org-link-parameters'?
>
> `(let ((org-link-parameters (copy-tree org-link-parameters)))
> (org-link-set-parameters ,type ,@parameters)
> ,@body))
I had the same thought. I doesn't work though. `copy-tree' and
`copy-sequence' only make shallow copies of each element in a
sequence. `org-link-set-parameters' then uses side effects to alter
the `org-link-parameters', so these changes are propagated to the
copy. This means the values in the copy and the real
`org-link-parameters' are always the same, and so we can't use the
copy to store the original values (which is what we need it to do).
> In addition, `declare' form should be added to `defmacro' to specify
> which argument is considered as its body.
I have never used `declare' before. I looked it up in the Elisp manual
and read the docstring, but I couldn't understand how it specifies
which argument is considered the body. I'm also not aware of what this
does/why this is useful? (not a criticism, I just haven't learned this
yet).
> My opinion is that it should be inside
> (let ((initial-input ...
This is a good idea -- done.
> and maybe even be a sibling of
> (def (org-link-get-parameter type
> to keep related code more local.
This isn't possible, because that's a clause in a `let' call, which is
itself inside a `cond' clause, and the `CONDITION' of that clause uses
`type', so it has to be defined at a higher level.
> I have not tested, so I may be wrong in respect to the following. It
> seems, you rises priority of desc value that earlier was a fallback.
> The reason why I am in doubts, is that it seems, desc is initialized
> from current link description when point is withing an existing link
> and in such cases desc likely should be preserved. I can not say
> that I like that it is responsibility of all description functions
> to return the desc argument if it is supplied.
It's right that `desc' is initialized from current link description
when point is withing an existing link.
Previously, `desc' was only used when
`org-link-make-description-function' was nil. This seems to me a very
odd behaviour: preserve the current link description, but only when
`org-link-make-description-function' is nil. Especially considering
that `org-insert-link' is also used for editing already-existing
links. So in the original version, in some situations,
`org-link-make-description-function' had a higher priority than
`desc', which seems wrong.
Thanks,
Hugo
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ol.el-add-description-format-parameter-to-org-link-p.patch --]
[-- Type: text/x-diff, Size: 6402 bytes --]
From aac4b04e8448436dd2713116870ee86d87fa2005 Mon Sep 17 00:00:00 2001
From: Hugo Heagren <hugo@heagren.com>
Date: Mon, 28 Mar 2022 23:18:45 +0100
Subject: [PATCH] ol.el: add description format parameter to
org-link-parameters
* ol.el (org-link-parameters): Add parameter `:insert-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 `:insert-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.
---
etc/ORG-NEWS | 7 +++++
lisp/ol.el | 73 ++++++++++++++++++++++++++++++++++++++--------------
2 files changed, 60 insertions(+), 20 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 35af94f92..86128d100 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -322,6 +322,13 @@ purpose of the variable. The replacement variable
accepts =listings= and =verbatim= in place of =t= and =nil= (which
still work, but are no longer listed as valid options).
+*** ~org-link-parameters~ has a new ~:insert-description~ parameter
+
+The value of ~:insert-description~ is used as the initial input when
+prompting for a link description. It can be a string (used as-is) or
+a function (called with the same arguments as
+~org-make-link-description-function~ to return a string to use).
+
* Version 9.5
** Important announcements and breaking changes
diff --git a/lisp/ol.el b/lisp/ol.el
index d4bd0e40c..5c702e33a 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -141,6 +141,18 @@ link.
Function that inserts a link with completion. The function
takes one optional prefix argument.
+`:insert-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'). If it returns nil, no
+ default description is used, but no error is thrown (from the
+ user's perspective, this is equivalent to a default description
+ of \"\").
+
`:display'
Value for `invisible' text property on the hidden parts of the
@@ -1804,11 +1816,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 `:insert-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 +1833,10 @@ don't allow to edit the default description."
(desc region)
(link link-location)
(abbrevs org-link-abbrev-alist-local)
- entry all-prefixes auto-desc)
+ (all-prefixes (append (mapcar #'car abbrevs)
+ (mapcar #'car org-link-abbrev-alist)
+ (org-link-types)))
+ entry auto-desc)
(cond
(link-location) ; specified by arg, just use it.
((org-in-regexp org-link-bracket-re 1)
@@ -1859,9 +1877,6 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
(unless (pos-visible-in-window-p (point-max))
(org-fit-window-to-buffer))
(and (window-live-p cw) (select-window cw))))
- (setq all-prefixes (append (mapcar #'car abbrevs)
- (mapcar #'car org-link-abbrev-alist)
- (org-link-types)))
(unwind-protect
;; Fake a link history, containing the stored links.
(let ((org-link--history
@@ -1958,17 +1973,35 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
(setq desc path)))))
(unless auto-desc
- (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))))))
+ (let* ((type
+ (cond
+ ((string-match (rx-to-string `(: string-start (submatch (or ,@all-prefixes)) ":")) link)
+ (match-string 1 link))
+ ((file-name-absolute-p link) "file")
+ ((string-match "\\`\\.\\.?/" link) "file")))
+ (initial-input
+ (cond
+ (description)
+ (desc)
+ ((org-link-get-parameter type :insert-description)
+ (let ((def (org-link-get-parameter type :insert-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 `:insert-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"
+ 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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-test-ol-tests-for-insert-description-param-when-inse.patch --]
[-- Type: text/x-diff, Size: 4204 bytes --]
From b8be9031c58a88e7d3294d587847b3edbbb43d36 Mon Sep 17 00:00:00 2001
From: Hugo Heagren <hugo@heagren.com>
Date: Sat, 16 Jul 2022 19:50:15 +0100
Subject: [PATCH] test-ol: tests for insert-description param when inserting
links
* test-ol (test-ol-with-link-parameters-as): Convenience macro for
testing.
(test-ol-insert-link-get-desc): Convenience macro for testing.
(test-ol/return-foobar): Convenience function for testing.
(test-ol/insert-link-insert-description): Test for various values of
`:insert-description' in `org-link-parameters' (including
`test-ol/return-foobar').
---
testing/lisp/test-ol.el | 88 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el
index 429bb52ee..d2af972bd 100644
--- a/testing/lisp/test-ol.el
+++ b/testing/lisp/test-ol.el
@@ -19,6 +19,8 @@
;;; Code:
+(require 'cl-lib)
+
\f
;;; Decode and Encode Links
@@ -625,5 +627,91 @@ 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'."
+ (let (orig-parameters)
+ ;; Copy all keys in `parameters' and their original values to
+ ;; `orig-parameters'.
+ (map-do
+ (lambda (key _) (setq orig-parameters
+ (plist-put
+ orig-parameters key
+ (org-link-get-parameter type key))))
+ parameters)
+ `(unwind-protect
+ ;; Set `parameters' values and execute body.
+ (progn (org-link-set-parameters ,type ,@parameters) ,@body)
+ ;; Restore original values.
+ (apply 'org-link-set-parameters ,type ',orig-parameters))))
+
+(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 `:insert-description'
+API in `org-link-parameters'. Used in test
+`test-ol/insert-link-insert-description', for the case where
+`:insert-description' is a function symbol."
+ "foobar")
+
+(ert-deftest test-ol/insert-link-insert-description ()
+ "Test `:insert-description' parameter handling."
+ ;; String case.
+ (should
+ (string=
+ "foobar"
+ (test-ol-with-link-parameters-as
+ "id" (:insert-description "foobar")
+ (test-ol-insert-link-get-desc "id:foo-bar"))))
+ ;; Lambda case.
+ (should
+ (string=
+ "foobar"
+ (test-ol-with-link-parameters-as
+ "id" (:insert-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" (:insert-description #'test-ol/return-foobar)
+ (test-ol-insert-link-get-desc "id:foo-bar"))))
+ ;; `:insert-description' parameter is defined, but doesn't return a
+ ;; string.
+ (should
+ (null
+ (test-ol-with-link-parameters-as
+ "id" (:insert-description #'ignore)
+ (test-ol-insert-link-get-desc "id:foo-bar"))))
+ ;; Description argument should override `:insert-description'.
+ (should
+ (string=
+ "notfoobar"
+ (test-ol-with-link-parameters-as
+ "id" (:insert-description "foobar")
+ (test-ol-insert-link-get-desc "id:foo-bar" "notfoobar")))))
+
(provide 'test-ol)
;;; test-ol.el ends here
--
2.20.1
next prev parent reply other threads:[~2022-07-16 21:16 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
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-07-07 19:57 ` [PATCH v4] " Hugo Heagren
2022-07-09 3:31 ` Ihor Radchenko
2022-07-14 13:08 ` [PATCH v5] " Hugo Heagren
2022-07-16 9:09 ` Ihor Radchenko
2022-07-16 21:20 ` Hugo Heagren [this message]
2022-07-17 6:11 ` Max Nikulin
2022-07-17 10:27 ` Ihor Radchenko
2022-07-17 10:18 ` Ihor Radchenko
2022-07-17 20:59 ` [PATCH v6] " Hugo Heagren
2022-07-18 10:55 ` Max Nikulin
2022-07-23 7:48 ` [PATCH v7] " Hugo Heagren
2022-07-23 7:59 ` Max Nikulin
2022-07-23 13:06 ` Ihor Radchenko
2022-07-23 15:46 ` Max Nikulin
2022-07-24 10:34 ` Max Nikulin
2022-07-24 13:15 ` Ihor Radchenko
2022-07-25 11:55 ` [PATCH v8] " Hugo Heagren
2022-07-29 12:54 ` Max Nikulin
2022-07-29 19:05 ` [PATCH v9] " Hugo Heagren
2022-07-30 6:29 ` Ihor Radchenko
[not found] ` <87tu6zf2o1.fsf@heagren.com>
2022-07-30 8:02 ` Ihor Radchenko
2022-07-30 12:34 ` [PATCH] ol-info: Enable :insert-description feature Max Nikulin
2022-08-06 7:00 ` Ihor Radchenko
2022-08-14 16:41 ` [PATCH v2] ol-info: Define :insert-description function Max Nikulin
2022-08-19 4:28 ` Ihor Radchenko
2022-08-19 12:26 ` Max Nikulin
2022-08-20 7:29 ` Ihor Radchenko
2022-08-21 14:49 ` Max Nikulin
2022-08-22 4:10 ` Ihor Radchenko
2022-08-24 14:37 ` [PATCH v3] " Max Nikulin
2022-08-26 13:15 ` Ihor Radchenko
2022-09-04 15:05 ` [PATCH] ol-info: Enable :insert-description feature Max Nikulin
2022-09-05 6:36 ` Ihor Radchenko
2022-08-06 6:06 ` [PATCH v9] ol.el: add description format parameter to org-link-parameters Ihor Radchenko
2022-07-29 1:47 ` [PATCH v7] " Ihor Radchenko
2022-07-29 7:05 ` Bastien Guerry
2022-07-10 10:26 ` [PATCH v4] " Max Nikulin
2022-06-21 15:01 ` [PATCH v3] " Max Nikulin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tu7gkb4l.fsf@heagren.com \
--to=hugo@heagren.com \
--cc=emacs-orgmode@gnu.org \
--cc=yantar92@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).