[PATCH v4] ol.el: add description format parameter to org-link-parameters
Since the last version of this patch, I have: - moved the code which sets `type' in `org-insert-link' to a position where it covers more cases - rewritten the macros used in the tests, so that always (and correctly) restore the original state after running, even after errors. Thanks to Max Nikulin for suggesting using `condition-case' There is only one thing left which I am not happy with. Currently, the test fails in the case where :default-description is 'ignore. This was intended to test for situations where the parameter is set to a function, but the function doesn't return a string (I used ignore because it returns 'nil). Accordingly, the test is a `should-error' (because the function *should* return a string, so we should error if it doesn't, right?). But the error condition is inside the error clause of a condition case---which is only triggered if the code errors. Calling 'ignore with any arguments and getting 'nil back doesn't cause any errors, so the error clause is never triggered, and 'nil is just used as the default description (which is then ignored, because it is 'nil -- other non-string values like numbers would not be ignored). I'd like some input on what to do about this: I could rewrite the tests so that a nil-value doesn't matter. In the case of this test, a non-interactive call would just insert a link without a description. Alternatively I could rewrite the function so that if the :default-description parameter returns something, we error unless it is a string. tl;dr The question is: what is the Good Behaviour when :default-description is set to something, which is meant to return a string and returns 'nil instead? Should it be treated like an empty string, or as an error? Thanks, Hugo >From ae17e87436def764f99b24add4debb5d7a481e1a Mon Sep 17 00:00:00 2001 From: Hugo Heagren 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 | 92 + 1 file changed, 92 insertions(+) diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el index 429bb52ee..9114c6497 100644 --- a/testing/lisp/test-ol.el +++ b/testing/lisp/test-ol.el @@ -625,5 +625,97 @@ See https://github.com/yantar92/org/issues/4.; (test-ol-parse-link-in-text "The http://foo.com/(something)?after=parens link" +;;; Insert Links + +(defmacro test-ol-with-link-parameters-as (type parameters 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'." + ;; Copy all keys in `parameters' and their original values to + ;; `orig-parameters'. For `parity': nil = odd, non-nill = even + `(let (parity orig-parameters) + (dolist (p ',parameters) + (unless parity + (setq orig-parameters + (plist-put orig-parameters p (org-link-get-parameter ,type p + (setq parity (not parity))) + ;; Set `parameters' values + (condition-case err + (let ((_ (org-link-set-parameters ,type ,@parameters)) + ;; Do body + (rtn (progn ,@body))) + ;; Restore original values + (apply 'org-link-set-parameters ,type orig-parameters) + ;; Return whatever the body returned + rtn) + ;; In case of error, restore state anyway AND really error + (error +(apply 'org-link-set-parameters ,type orig-parameters) +(signal (car err) (cdr err)) + +(defun test-ol-insert-link-get-desc ( 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-inse
Re: [PATCH v3] ol.el: add description format parameter to org-link-parameters
I've finally had time to the `:default-description' parameter in `org-link-parameters'. Ihor Radchenko 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 " %S: "', since %S will print a symbol, a string or a lambda correctly. Hope this is a bit better! Best, Hugo >From 5a44edfda4e09a95bba1327c2758b2637e5b16bd Mon Sep 17 00:00:00 2001 From: Hugo Heagren 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 http://foo.com/(something)?after=parens link" +;;; Insert Links + +(defmacro test-ol-with-link-parameters-as (type parameters 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 ( 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 `
[PATCH v2] 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. --- 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
[PATCH] 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 | 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
ol.el: add description format parameter to org-link-parameters
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
[BUG] beamer export -- text inside section, before next headline, has no frame [9.5.2 (9.5.2-ge7ea95 @ /home/hugo/.config/emacs/straight/build/org/)]
Remember to cover the basics, that is, what you expected to happen and
what in fact did happen. You don't know how to make a good
report? See
https://orgmode.org/manual/Feedback.html#Feedback
Your bug report will be posted to the Org mailing list.
-
---
I have org-beamer-frame-level set to 2, so when exporting to beamer,
top-level headlines become sections, and second-level headlines become
frames. I often want to include a short explanation/intro of the
section, before moving onto the more specific slide (I have found this
is good for complex lectures).
So I have something like:
```org
* A section
This is in the section, but not in the subsection (and is therefore
not in a frame, and shows up at the top of the slide).
** A subsection
Some text in the subsection. This /is/ in a frame, and so behaves
correctly.
```
As the MWE says, the text inside the top-level (section) headline, but
before the next (frame) headline, is not wrapped in a frame environment
when exported, so it renders strangely (at the top of the slide).
I feel like this is an easy fix for someone who understands the
exporter
code well. I've had a look and it's very complex, but I would be
grateful if someone else could try.
emacs : GNU Emacs 29.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version
3.24.5, cairo version 1.16.0)
of 2022-01-15
Package: Org mode version 9.5.2 (9.5.2-ge7ea95 @
/home/hugo/.config/emacs/straight/build/org/)
Blue skies, Hugo
Appendix.
-
I tested the above MWE with the following minimal init.el and
reproduced the bug (well, it's minimal for me, since I'm used to using
straight and use-package, and didn't want to learn any other way of
installing all this stuff just to check a bug):
```elisp
(defvar bootstrap-version)
(let ((bootstrap-file
(expand-file-name "straight/repos/straight.el/bootstrap.el"
user-emacs-directory))
(bootstrap-version 5))
(unless (file-exists-p bootstrap-file)
(with-current-buffer
(url-retrieve-synchronously
"
https://raw.githubusercontent.com/raxod502/straight.el/develop/install.el
"
'silent 'inhibit-cookies)
(goto-char (point-max))
(eval-print-last-sexp)))
(load bootstrap-file nil 'nomessage))
;; Always integrate straight.el with use-package
(setq straight-use-package-by-default t)
;; Actually INSTALL use-package
(straight-use-package 'use-package)
(use-package org
:straight (:includes ox-beamer))
(use-package ox-beamer
:after org
:config
(add-to-list 'org-latex-classes
`("beamer"
,(concat "\\documentclass[presentation]{beamer}\n"
"[DEFAULT-PACKAGES]"
"[PACKAGES]"
"[EXTRA]\n")
("\\section{%s}" . "\\section*{%s}")
("\\subsection{%s}" . "\\subsection*{%s}")
("\\subsubsection{%s}" . "\\subsubsection*{%s}")))
:custom
(org-beamer-frame-level 2))
```
Suggestion: convert dispatchers to use transient
Org uses various dispatchers, where invoking a command gives the user a choice of different sub-commands, chosen by pressing a relevant key, from a list displayed on the screen. Some of these dispatchers include options which can affect the command chosen. Examples include org- capture, org-beamer-select-environment and org-export-dispatch. These desptachers are idiosyncratic, written for purpose, and each behave differently. They have varying levels of customisability, and this is reached in different ways for each. Overall, I think the user- experience could be more consistent and more easily customisable. Luckily, recent versions of emacs ship with transient.el, a powerful way of building such interfaces in a consistent and easily extensible way. So, I propose to rewrite the current dispatchers as transients. What does the community think? I would be happy to work on this unless others strongly object, but I don't know everything about org, so if others could help me with a list of other dispatchers which could also be converted that would be helpful. Blue skies, Hugo
