[PATCH] ol.el: Always prompt for description in `org-insert-link'

[PATCH] ol.el: Always prompt for description in `org-insert-link'

[Max Nikulin]

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

Hi,

The `org-insert-link' function has the `auto-desc' variable to suppress 
user prompt for link description. It worked accordingly its intention 
(as I understand it) a couple of months a decade ago, after that the 
logic was broken. Actually the behavior is rather strange: users may 
edit description only when it is not identical to link path (target). I 
am attaching a patch that removes this variable with hope that always 
prompting the user for description is more consistent behavior.

Actually during last week completion stored link by its description has 
been restored, storing link has been changed to prefer nil as 
description to the path when nothing meaningful may be added. So 
behavior have become more close to original one.

I have an idea how to fix the code to bypass the description prompt if 
path is completed by description discarding identical path and 
description, but I prefer to drop the confusing variable and not depend 
on the means how the link was selected from the list of stored items.

Have I missed something and unconditional prompt to edit link 
description may be undesired in some cases?

[-- Attachment #2: 0001-ol.el-Always-prompt-for-description-in-org-insert-li.patch --]
[-- Type: text/x-patch, Size: 5963 bytes --]

From b4e5195da4f084a9efd9697db556168d9907a17e Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Wed, 14 Sep 2022 22:43:51 +0700
Subject: [PATCH 1/2] ol.el: Always prompt for description in `org-insert-link'

* lisp/ol.el (org-insert-link): Do not bypass code trying to generated
description and prompt user when link path and description are
identical.  Make behavior of description prompt more consistent.

Remove confusing `auto-desc' local variable.  Originally the variable
was added with implementation of completion of stored link target by the
description in the commit 1e34c5d34 Bastien Guerry, "org.el: Fontify
links to current buffer when inserting a link",
2012-08-03 14:08:20 +0200.  The feature was broken soon by the commit
7f096ad37 Tony Day, "org-insert-link: Use ido when inserting links",
2012-10-12 14:39:53 +1100.  Last decade users were not asked to edit
description in the case of the same link target and description
(a remained side effect of 1e34c5d34).  Recent commit 0432f4fe6 Max
Nikulin, "ol.el: Restore complete by description for insert link",
2022-09-10 17:23:13 +0700 restored completion by description.
Due to the commit 4fc2c8dd8 Ihor Radchenko, "org-store-link: Default to
empty description for target/custom-id links", 2022-08-10 13:25:26 +0800
description identical to link path became a more rare case.

An alternative would be fixing condition to allow users to edit
description when it is the same as the path, but use stored description
without additional interaction when the link is chosen by description
completion.  Despite it was likely the original intention, always asking
the user to confirm or edit description may be more consistent behavior.
---
 lisp/ol.el | 72 ++++++++++++++++++++++++++----------------------------
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index f3f6e04ef..8f700397e 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -1822,7 +1822,7 @@ non-interactively, don't allow to edit the default description."
 	 (all-prefixes (append (mapcar #'car abbrevs)
 			       (mapcar #'car org-link-abbrev-alist)
 			       (org-link-types)))
-         entry auto-desc)
+         entry)
     (cond
      (link-location)		      ; specified by arg, just use it.
      ((org-in-regexp org-link-bracket-re 1)
@@ -1885,8 +1885,7 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 	    (unless (org-string-nw-p link) (user-error "No link selected"))
 	    (dolist (l org-stored-links)
 	      (when (equal link (cadr l))
-		(setq link (car l))
-		(setq auto-desc t)))
+		(setq link (car l))))
 	    (when (or (member link all-prefixes)
 		      (and (equal ":" (substring link -1))
 			   (member (substring link 0 -1) all-prefixes)
@@ -1963,41 +1962,40 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 	  (when (equal desc origpath)
 	    (setq desc path)))))
 
-    (unless auto-desc
-      (let* ((type
-              (cond
-               ((and all-prefixes
-                     (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))))
-	       (org-link-make-description-function
+    (let* ((type
+            (cond
+             ((and all-prefixes
+                   (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
-		    (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))))
+                    (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))))
+             (org-link-make-description-function
+              (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)))
 
     (unless (org-string-nw-p desc) (setq desc nil))
     (when remove (apply #'delete-region remove))
-- 
2.25.1


[-- Attachment #3: 0002-ol.el-Mention-that-org-insert-link-may-edit-existing.patch --]
[-- Type: text/x-patch, Size: 1036 bytes --]

From e5d1cb2656bbd3f5026c4ab5661931b40d14c767 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Wed, 14 Sep 2022 22:55:28 +0700
Subject: [PATCH 2/2] ol.el: Mention that `org-insert-link' may edit existing
 link

* lisp/ol.el (org-insert-link): Add completion by description and edit
link features to the docstring.
---
 lisp/ol.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/ol.el b/lisp/ol.el
index 8f700397e..39d644025 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -1777,6 +1777,9 @@ The history can be used to select a link previously stored with
 press `RET' at the prompt), the link defaults to the most recently
 stored link.  As `SPC' triggers completion in the minibuffer, you need to
 use `M-SPC' or `C-q SPC' to force the insertion of a space character.
+Completion candidates include link descriptions.
+
+If there is a link under cursor then edit it.
 
 You will also be prompted for a description, and if one is given, it will
 be displayed in the buffer instead of the link.
-- 
2.25.1

Re: [PATCH] ol.el: Always prompt for description in `org-insert-link'

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> I have an idea how to fix the code to bypass the description prompt if 
> path is completed by description discarding identical path and 
> description, but I prefer to drop the confusing variable and not depend 
> on the means how the link was selected from the list of stored items.
>
> Have I missed something and unconditional prompt to edit link 
> description may be undesired in some cases?

AFAIU, this particular behaviour is undocumented. If it is used by some
other code, let's make tests for it, document, and fix in a better, more
explicit way.

I am unaware about the code using this link==description behaviour.

Applied onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=69b36beac790bad95fdd9ce4a7bcfbbb46d39c64
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6d8d7fba61652434764c9906df69db373ff4f7f6

Bastien,

Could you please check the ongoing changes in this area
(storing/inserting links)? The code in this area is old, confusing, and
not fully documented. I'm afraid that we may break something in the
process of refactoring. If you see some dangerous changes, please let us
know.

-- 
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92

Re: [PATCH] ol.el: Always prompt for description in `org-insert-link'

[Bastien Guerry]

Ihor Radchenko <yantar92@gmail.com> writes:

> Could you please check the ongoing changes in this area
> (storing/inserting links)? The code in this area is old, confusing, and
> not fully documented. I'm afraid that we may break something in the
> process of refactoring. If you see some dangerous changes, please let us
> know.

Thanks for the heads up, I'll have a look this week.

> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=69b36beac790bad95fdd9ce4a7bcfbbb46d39c64
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6d8d7fba61652434764c9906df69db373ff4f7f6

Are these changes the commits I should start from?  If there are other
important changes, let me know.

Thanks,

-- 
 Bastien

Re: [PATCH] ol.el: Always prompt for description in `org-insert-link'

[Ihor Radchenko]

Bastien Guerry <bzg@gnu.org> writes:

>> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=69b36beac790bad95fdd9ce4a7bcfbbb46d39c64
>> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6d8d7fba61652434764c9906df69db373ff4f7f6
>
> Are these changes the commits I should start from?  If there are other
> important changes, let me know.

The relevant commits are:

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=ac2d0a249e5419fb52bc1e953e70c847a31d40de
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=33686b9955c011486a72ec548475651dcadb0339
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6e9ea3a0766de9df6992bfb0359a5826ed1951b5
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f25b308af671766bb01e507c6db57bf3e83d8d05
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=0432f4fe6ba9b07c17ac555beab1527d8f844234
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=543a23a57d2947cd01c906d134ae9c5c8d0907c4
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f7b8510283537bda4eba3b54fce5eafc7cec9993
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c3d6672cfdbff8c9dd4c2ec70886ad3f62153d07
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4fc2c8dd89bfbe9f6ad7620c1b4d6def4114489b
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=69b36beac790bad95fdd9ce4a7bcfbbb46d39c64
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6d8d7fba61652434764c9906df69db373ff4f7f6

-- 
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92