* Bug: Fix for org-make-link-description-function use in org-insert-link [9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)]
@ 2017-09-05 16:27 Reuben Thomas
2017-09-05 20:49 ` Nicolas Goaziou
0 siblings, 1 reply; 6+ messages in thread
From: Reuben Thomas @ 2017-09-05 16:27 UTC (permalink / raw)
To: emacs-orgmode
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
http://orgmode.org/manual/Feedback.html#Feedback
Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------
I was trying to make link descriptions default to the link. This is
useful so that, for example, in LaTeX export, links without a
description do not turn into empty text if one has unnumbered sections.
Also, it’s useful if one is primarily using PDF output, with hyperlinks.
I used the following code:
(defun org-make-link-description (link desc)
(or desc link))
(setq org-make-link-description-function #'org-make-link-description)
This works; however, no description is prompted for (the description is
*always* the default).
This seems to be incorrect behaviour, as the docstring for
org-insert-link says:
If `org-make-link-description-function' is non-nil, this function will be
called with the link target, and the result will be the default
link description.
The implication is that the value returned is used as the default, not
that it overrides the prompt.
The current logic dates from commit d9e5aed2, which aims to fix a
previous bug where the prompt was not offered if
org-make-link-description-function failed. The commit message implies
that the prompt should not be offered if
org-make-link-description-function succeeds, but I believe that is
wrong.
Below is a patch which both corrects the behaviour and simplifies the
logic.
First, org-make-link-description-function is called if it is non-nil,
and used to set default-description.
Then (unless auto-desc is non-nil) the description is prompted for with
default-description as the default value (unless
org-make-link-description-function returned nil, in which case the
current value of desc, if any, is used).
There is one further matter that my patch does not address: the
docstring for org-make-link-description-function says:
“When [org-make-link-description-function is] nil, the link location
will be used.”
This does not happen (it’s precisely the behaviour I’ve been trying to
obtain!), and my patch does not make it happen. Indeed, it’s not clear
that it’s desirable (if one is using numbered sections, for example).
Patch follows:
Subject: [PATCH] Fix logic of calling org-make-link-desciption-function
* lisp/org.el (org-insert-link): Simplify so that description is only
prompted for once, and org-make-link-description-function obviously
sets the default description, as per the docstring. Then, always
prompt for the description, even if the default is set.
---
lisp/org.el | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/lisp/org.el b/lisp/org.el
index 2680cee..0731cf7 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -10286,16 +10286,15 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
(setq desc path)))))
(if org-make-link-description-function
- (setq desc
- (or (condition-case nil
- (funcall org-make-link-description-function link desc)
- (error (progn (message "Can't get link description from `%s'"
- (symbol-name org-make-link-description-function))
- (sit-for 2) nil)))
- (read-string "Description: " default-description)))
- (if default-description (setq desc default-description)
- (setq desc (or (and auto-desc desc)
- (read-string "Description: " desc)))))
+ (setq default-description
+ (condition-case nil
+ (funcall org-make-link-description-function link desc)
+ (error (progn (message "Can't get link description from `%s'"
+ (symbol-name org-make-link-description-function))
+ (sit-for 2) nil)))))
+
+ (setq desc (or (and auto-desc desc)
+ (read-string "Description: " (or default-description desc))))
(unless (string-match "\\S-" desc) (setq desc nil))
(when remove (apply 'delete-region remove))
--
2.7.4
Emacs : GNU Emacs 25.2.1 (x86_64-pc-linux-gnu, GTK+ Version 3.18.9)
of 2017-04-29
Package: Org mode version 9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)
current state:
==============
(setq
org-tab-first-hook '(org-babel-hide-result-toggle-maybe org-babel-header-arg-expand)
org-latex-classes '(("article" "\\documentclass[11pt]{article}" ("\\section{%s}" . "\\section*{%s}")
("\\subsection{%s}" . "\\subsection*{%s}") ("\\subsubsection{%s}" . "\\subsubsection*{%s}")
("\\paragraph{%s}" . "\\paragraph*{%s}") ("\\subparagraph{%s}" . "\\subparagraph*{%s}"))
("report" "\\documentclass[11pt]{report}" ("\\part{%s}" . "\\part*{%s}")
("\\chapter{%s}" . "\\chapter*{%s}") ("\\section{%s}" . "\\section*{%s}")
("\\subsection{%s}" . "\\subsection*{%s}") ("\\subsubsection{%s}" . "\\subsubsection*{%s}"))
("book" "\\documentclass[11pt]{book}" ("\\part{%s}" . "\\part*{%s}")
("\\chapter{%s}" . "\\chapter*{%s}") ("\\section{%s}" . "\\section*{%s}")
("\\subsection{%s}" . "\\subsection*{%s}") ("\\subsubsection{%s}" . "\\subsubsection*{%s}"))
("scrartcl" "\\documentclass{scrartcl}" ("\\section{%s}" . "\\section*{%s}")
("\\subsection{%s}" . "\\subsection*{%s}") ("\\subsubsection{%s}" . "\\subsubsection*{%s}")
("\\paragraph{%s}" . "\\paragraph*{%s}") ("\\subparagraph{%s}" . "\\subparagraph*{%s}"))
)
org-latex-default-packages-alist '(("postscript" "ucs" nil) ("" "pifont" nil) ("utf8x" "inputenc" t) ("T1" "fontenc" t)
("" "fixltx2e" nil) ("" "graphicx" t) ("" "longtable" nil) ("" "float" nil)
("" "wrapfig" nil) ("" "rotating" nil) ("normalem" "ulem" t) ("" "amsmath" t)
("" "textcomp" t) ("" "marvosym" t) ("" "wasysym" t) ("" "amssymb" t)
("" "hyperref" nil) "\\tolerance=1000" ("" "palatino" nil))
org-texinfo-format-drawer-function #[514 "\207" [] 3 "\n\n(fn NAME CONTENTS)"]
org-speed-command-hook '(org-speed-command-default-hook org-babel-speed-command-hook)
org-occur-hook '(org-first-headline-recenter)
org-metaup-hook '(org-babel-load-in-session-maybe)
org-html-format-drawer-function #[514 "\207" [] 3 "\n\n(fn NAME CONTENTS)"]
org-latex-format-inlinetask-function 'org-latex-format-inlinetask-default-function
org-confirm-shell-link-function 'yes-or-no-p
org-ascii-format-inlinetask-function 'org-ascii-format-inlinetask-default
org-startup-folded nil
org-export-with-section-numbers nil
org-file-apps '((t . "xdg-open %s") (system . "xdg-open %s"))
org-export-with-toc nil
org-latex-format-headline-function 'org-latex-format-headline-default-function
org-default-notes-file "~/Notes/Notes.org"
org-startup-indented t
org-after-todo-state-change-hook '(org-clock-out-if-current)
org-latex-format-drawer-function #[514 "\207" [] 3 "\n\n(fn _ CONTENTS)"]
org-odt-format-headline-function 'org-odt-format-headline-default-function
org-from-is-user-regexp "\\<Reuben Thomas\\>"
org-src-mode-hook '(org-src-babel-configure-edit-buffer org-src-mode-configure-edit-buffer)
org-agenda-before-write-hook '(org-agenda-add-entry-text)
org-babel-pre-tangle-hook '(save-buffer)
org-mode-hook '((lambda nil (local-set-key "\351" (quote org-toggle-iimage-in-org))) window-margin-mode
#[0 "\300\301\302\303\304$\207" [add-hook change-major-mode-hook org-show-block-all append local] 5]
#[0 "\300\301\302\303\304$\207" [add-hook change-major-mode-hook org-babel-show-result-all append local] 5]
org-babel-result-hide-spec org-babel-hide-all-hashes turn-on-eldoc-mode)
org-texinfo-format-headline-function 'org-texinfo-format-headline-default-function
org-archive-hook '(org-attach-archive-delete-maybe)
org-make-link-description-function 'org-make-link-description
org-ascii-format-drawer-function #[771 "\207" [] 4 "\n\n(fn NAME CONTENTS WIDTH)"]
org-odt-format-inlinetask-function 'org-odt-format-inlinetask-default-function
org-directory "~/Notes"
org-cycle-hook '(org-cycle-hide-archived-subtrees org-cycle-hide-drawers org-cycle-show-empty-lines
org-optimize-window-after-visibility-change)
org-footnote-define-inline t
org-confirm-elisp-link-function 'yes-or-no-p
org-metadown-hook '(org-babel-pop-to-session-maybe)
org-odt-format-drawer-function #[514 "\207" [] 3 "\n\n(fn NAME CONTENTS)"]
org-latex-default-class "scrartcl"
org-html-format-headline-function 'org-html-format-headline-default-function
org-link-parameters '(("id" :follow org-id-open) ("rmail" :follow org-rmail-open :store org-rmail-store-link)
("mhe" :follow org-mhe-open :store org-mhe-store-link)
("irc" :follow org-irc-visit :store org-irc-store-link)
("info" :follow org-info-open :export org-info-export :store org-info-store-link)
("gnus" :follow org-gnus-open :store org-gnus-store-link)
("docview" :follow org-docview-open :export org-docview-export :store org-docview-store-link)
("bibtex" :follow org-bibtex-open :store org-bibtex-store-link)
("bbdb" :follow org-bbdb-open :export org-bbdb-export :complete org-bbdb-complete-link :store
org-bbdb-store-link)
("w3m" :store org-w3m-store-link) ("file+sys") ("file+emacs") ("doi" :follow org--open-doi-link)
("elisp" :follow org--open-elisp-link) ("file" :complete org-file-complete-link)
("ftp" :follow (lambda (path) (browse-url (concat "ftp:" path))))
("help" :follow org--open-help-link)
("http" :follow (lambda (path) (browse-url (concat "http:" path))))
("https" :follow (lambda (path) (browse-url (concat "https:" path))))
("mailto" :follow (lambda (path) (browse-url (concat "mailto:" path))))
("message" :follow (lambda (path) (browse-url (concat "message:" path))))
("news" :follow (lambda (path) (browse-url (concat "news:" path))))
("shell" :follow org--open-shell-link))
org-footnote-auto-label nil
org-html-format-inlinetask-function 'org-html-format-inlinetask-default-function
org-agenda-files '("~/Writing/Ithaky/Calendar.org")
org-texinfo-format-inlinetask-function 'org-texinfo-format-inlinetask-default-function
org-clock-out-hook '(org-clock-remove-empty-clock-drawer)
org-latex-inputenc-alist '(("utf8" . "utf8x"))
)
--
http://rrt.sc3d.org/
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Bug: Fix for org-make-link-description-function use in org-insert-link [9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)]
2017-09-05 16:27 Bug: Fix for org-make-link-description-function use in org-insert-link [9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)] Reuben Thomas
@ 2017-09-05 20:49 ` Nicolas Goaziou
2017-09-05 21:05 ` Reuben Thomas
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Goaziou @ 2017-09-05 20:49 UTC (permalink / raw)
To: Reuben Thomas; +Cc: emacs-orgmode
Hello,
Reuben Thomas <rrt@sc3d.org> writes:
> This seems to be incorrect behaviour, as the docstring for
> org-insert-link says:
>
> If `org-make-link-description-function' is non-nil, this function will be
> called with the link target, and the result will be the default
> link description.
>
> The implication is that the value returned is used as the default, not
> that it overrides the prompt.
I agree.
> First, org-make-link-description-function is called if it is non-nil,
> and used to set default-description.
I think the optional parameter from the function call should still
prevail. I.e., shouldn't this function be called when
DEFAULT-DESCRIPTION is nil (e.g., called interatively)?
> Then (unless auto-desc is non-nil) the description is prompted for with
> default-description as the default value (unless
> org-make-link-description-function returned nil, in which case the
> current value of desc, if any, is used).
>
> There is one further matter that my patch does not address: the
> docstring for org-make-link-description-function says:
>
> “When [org-make-link-description-function is] nil, the link location
> will be used.”
>
> This does not happen (it’s precisely the behaviour I’ve been trying to
> obtain!), and my patch does not make it happen. Indeed, it’s not clear
> that it’s desirable (if one is using numbered sections, for example).
Obviously, it would not be desirable.
However, I think it should read "When
`org-make-link-description-function' _returns_ nil, the link location is
used". This is not what is implemented either, tho.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: Fix for org-make-link-description-function use in org-insert-link [9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)]
2017-09-05 20:49 ` Nicolas Goaziou
@ 2017-09-05 21:05 ` Reuben Thomas
2017-09-05 22:02 ` Nicolas Goaziou
0 siblings, 1 reply; 6+ messages in thread
From: Reuben Thomas @ 2017-09-05 21:05 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode@gnu.org
[-- Attachment #1: Type: text/plain, Size: 2464 bytes --]
On 5 September 2017 at 21:49, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> Hello,
>
Hi, thanks for looking at this.
> Reuben Thomas <rrt@sc3d.org> writes:
>
> > This seems to be incorrect behaviour, as the docstring for
> > org-insert-link says:
> >
> > If `org-make-link-description-function' is non-nil, this function
> will be
> > called with the link target, and the result will be the default
> > link description.
> >
> > The implication is that the value returned is used as the default, not
> > that it overrides the prompt.
>
> I agree.
>
> > First, org-make-link-description-function is called if it is non-nil,
> > and used to set default-description.
>
> I think the optional parameter from the function call should still
> prevail. I.e., shouldn't this function be called when
> DEFAULT-DESCRIPTION is nil (e.g., called interatively)?
>
This question occurred to me too, but I didn't form an opinion. However, I
am inclined to agree with you on thinking about it some more.
> > Then (unless auto-desc is non-nil) the description is prompted for with
> > default-description as the default value (unless
> > org-make-link-description-function returned nil, in which case the
> > current value of desc, if any, is used).
> >
> > There is one further matter that my patch does not address: the
> > docstring for org-make-link-description-function says:
> >
> > “When [org-make-link-description-function is] nil, the link location
> > will be used.”
> >
> > This does not happen (it’s precisely the behaviour I’ve been trying to
> > obtain!), and my patch does not make it happen. Indeed, it’s not clear
> > that it’s desirable (if one is using numbered sections, for example).
>
> Obviously, it would not be desirable.
>
> However, I think it should read "When
> `org-make-link-description-function' _returns_ nil, the link location is
> used". This is not what is implemented either, tho.
>
If you (or someone) can confirm your interpretation above, I would be
happy to update my patch to implement the two behaviours required, namely,
that org-make-link-description-function is only called if the
default-description argument to org-insert-link is nil, and that if that
function returns nil, then the link location is used. I would also clarify
the docstring regarding the second behaviour.
--
https://rrt.sc3d.org <http://rrt.sc3d.org>
[-- Attachment #2: Type: text/html, Size: 3791 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: Fix for org-make-link-description-function use in org-insert-link [9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)]
2017-09-05 21:05 ` Reuben Thomas
@ 2017-09-05 22:02 ` Nicolas Goaziou
2017-09-07 23:06 ` Reuben Thomas
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Goaziou @ 2017-09-05 22:02 UTC (permalink / raw)
To: Reuben Thomas; +Cc: emacs-orgmode@gnu.org
Reuben Thomas <rrt@sc3d.org> writes:
> If you (or someone) can confirm your interpretation above, I would be
> happy to update my patch to implement the two behaviours required, namely,
> that org-make-link-description-function is only called if the
> default-description argument to org-insert-link is nil
Ack.
> and that if that function returns nil, then the link location is used.
> I would also clarify the docstring regarding the second behaviour.
It may not be a terribly useful behaviour anyway. You can always use
(lambda (link description) link) as
`org-make-link-description-function'.
Perhaps we can simply remove "When nil, the link location will be used"
from the docstring. Your call.
Thank you.
Regards,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: Fix for org-make-link-description-function use in org-insert-link [9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)]
2017-09-05 22:02 ` Nicolas Goaziou
@ 2017-09-07 23:06 ` Reuben Thomas
2017-09-08 5:00 ` Nicolas Goaziou
0 siblings, 1 reply; 6+ messages in thread
From: Reuben Thomas @ 2017-09-07 23:06 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode@gnu.org
[-- Attachment #1.1: Type: text/plain, Size: 933 bytes --]
On 5 September 2017 at 23:02, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:
> Reuben Thomas <rrt@sc3d.org> writes:
>
> > If you (or someone) can confirm your interpretation above, I would be
> > happy to update my patch to implement the two behaviours required,
> namely,
> > that org-make-link-description-function is only called if the
> > default-description argument to org-insert-link is nil
>
> Ack.
>
> > and that if that function returns nil, then the link location is used.
> > I would also clarify the docstring regarding the second behaviour.
>
> It may not be a terribly useful behaviour anyway. You can always use
> (lambda (link description) link) as
> `org-make-link-description-function'.
>
> Perhaps we can simply remove "When nil, the link location will be used"
> from the docstring. Your call.
>
I've removed it.
Attached, a revised patch.
--
https://rrt.sc3d.org
[-- Attachment #1.2: Type: text/html, Size: 1736 bytes --]
[-- Attachment #2: 0001-Fix-logic-of-calling-org-make-link-desciption-functi.patch --]
[-- Type: text/x-patch, Size: 3928 bytes --]
From df6e155c0e14bae6cebb2e8ac904405163c32b7d Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Tue, 5 Sep 2017 17:00:25 +0100
Subject: [PATCH] Fix logic of calling org-make-link-desciption-function
* lisp/org.el (org-insert-link): Simplify so that description is only
prompted for once, if auto-desc is not set, and takes as its default
value, in order, default-description, the return value of
org-make-link-description-function (if the variable is non-nil), and
the current desc. Update the docstring to reflect that
default-description takes precedence over
org-make-link-description-function.
(org-make-link-description-function): Remove from docstring the
statement that if the variable is nil, then the link will be used as
the default description. This is undesirable, and was not in any case
implemented.
---
lisp/org.el | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/lisp/org.el b/lisp/org.el
index 889987c..4347111 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1939,10 +1939,10 @@ in the Org buffer so that the change takes effect."
(defcustom org-make-link-description-function nil
"Function to use for generating link descriptions from links.
-When nil, the link location will be used. This function must take
-two parameters: the first one is the link, the second one is the
-description generated by `org-insert-link'. The function should
-return the description to use."
+This function must take two parameters: the first one is the
+link, the second one is the description generated by
+`org-insert-link'. The function should return the description to
+use."
:group 'org-link
:type '(choice (const nil) (function)))
@@ -10152,15 +10152,14 @@ the current directory or below.
A `\\[universal-argument] \\[universal-argument] \\[universal-argument]' \
prefix negates `org-keep-stored-link-after-insertion'.
-If `org-make-link-description-function' is non-nil, this function will be
-called with the link target, and the result will be the default
-link description.
-
If the LINK-LOCATION parameter is non-nil, this value will be used as
the link location instead of reading one interactively.
-If the DEFAULT-DESCRIPTION parameter is non-nil, this value will be used
-as the default description."
+If the DEFAULT-DESCRIPTION parameter is non-nil, this value will
+be used as the default description. Otherwise, if
+`org-make-link-description-function' is non-nil, this function
+will be called with the link target, and the result will be the
+default link description."
(interactive "P")
(let* ((wcf (current-window-configuration))
(origbuf (current-buffer))
@@ -10294,17 +10293,17 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
(when (equal desc origpath)
(setq desc path)))))
- (if org-make-link-description-function
- (setq desc
- (or (condition-case nil
- (funcall org-make-link-description-function link desc)
- (error (progn (message "Can't get link description from `%s'"
- (symbol-name org-make-link-description-function))
- (sit-for 2) nil)))
- (read-string "Description: " default-description)))
- (if default-description (setq desc default-description)
- (setq desc (or (and auto-desc desc)
- (read-string "Description: " desc)))))
+ (unless auto-desc
+ (setq desc (read-string "Description: "
+ (or default-description
+ (when org-make-link-description-function
+ (condition-case nil
+ (funcall org-make-link-description-function link desc)
+ (error (progn (message "Can't get link description from `%s'"
+ (symbol-name org-make-link-description-function))
+ (sit-for 2) nil))))
+
+ desc))))
(unless (string-match "\\S-" desc) (setq desc nil))
(when remove (apply 'delete-region remove))
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Bug: Fix for org-make-link-description-function use in org-insert-link [9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)]
2017-09-07 23:06 ` Reuben Thomas
@ 2017-09-08 5:00 ` Nicolas Goaziou
0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Goaziou @ 2017-09-08 5:00 UTC (permalink / raw)
To: Reuben Thomas; +Cc: emacs-orgmode@gnu.org
Hello,
Reuben Thomas <rrt@sc3d.org> writes:
> Attached, a revised patch.
Applied, with a minor refactoring so some lines could fit under 80
columns.
Thank you!
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-08 5:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05 16:27 Bug: Fix for org-make-link-description-function use in org-insert-link [9.0.10 (9.0.10-5-g1654a5-elpa @ /home/rrt/.emacs.d/elpa/org-20170904/)] Reuben Thomas
2017-09-05 20:49 ` Nicolas Goaziou
2017-09-05 21:05 ` Reuben Thomas
2017-09-05 22:02 ` Nicolas Goaziou
2017-09-07 23:06 ` Reuben Thomas
2017-09-08 5:00 ` Nicolas Goaziou
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).