Adding target and custom id links doesn't ask for description

Adding target and custom id links doesn't ask for description

[Carlos Pita]

Hi all,

When storing a <<target>> or CUSTOM_ID link with org-store-link and then adding it with C-c C-l, org doesn’t ask for a description but automatically inserts an ugly description like:

    ~/Desktop/Org/captures.org::target

or:

    ~/Desktop/Org/captures.org::#7EFBCEA1-0D17-45B9-A125-10FE581752F8

The entire path is inserted even when the target itself is simply “target”. But, more importantly, no chance to change the description is given to the user.

This behavior is not observed when an ID link is stored, which seems pretty inconsistent since there is no clear difference wrt the CUSTOM_ID case.

The inconvenience of this behavior and the inconsistent treatment of ID vs CUSTOM_ID makes me believe that this is not intended but a bug (perhaps a regression, but I’ve not been using org-mode for long enough to say).

The source of this behavior is in org-store-link:

        ;; If store function actually set `:description' property, use
        ;; it, even if it is nil.  Otherwise, fallback to link value.
	(setq desc (if (plist-member org-store-link-plist :description)
                       (plist-get org-store-link-plist :description)
		     link)))

so that auto-desc is later assumed in org-insert-link, even then the store function has provided no description at all!

Best regards
—
Carlos

Re: Adding target and custom id links doesn't ask for description

[Carlos Pita]

A few more details.

My org-mode version 9.5.4 (9.5.4-ge0b05b).

This is the link that org-insert-link inserts for <<here>>:

    [[here][file:~/Desktop/Org/capturas.org::here]]

This is the link that org-insert-link inserts for CUSTOM_ID = cusid:

    [[#cusid][file:~/Desktop/Org/capturas.org::#cusid]]

Perhaps description and link are swapped? In both cases the full path is redundant since the link is pointing to the current file, but the verbosity is arguably less adequate for the description field.

Best regards
—
Carlos

Re: Adding target and custom id links doesn't ask for description

[Max Nikulin]

On 03/08/2022 01:58, Carlos Pita wrote:
> 
> This is the link that org-insert-link inserts for <<here>>:
> 
>      [[here][file:~/Desktop/Org/capturas.org::here]]
> 
> This is the link that org-insert-link inserts for CUSTOM_ID = cusid:
> 
>      [[#cusid][file:~/Desktop/Org/capturas.org::#cusid]]
> 
> Perhaps description and link are swapped? In both cases the full path
> is redundant since the link is pointing to the current file, but the 
> verbosity is arguably less adequate for the description field.

In my opinion, attempt to fix storing of <<target>> links broke such use 
cases, however I am unsure that we are complaining about exactly the 
same things. My attempts to alleviate the issue:

Max Nikulin. Re: Bug: org-store-link uses CUSTOM_ID instead of target 
point. Sat, 6 Nov 2021 19:51:29 +0700. 
https://list.orgmode.org/e2c807a7-1924-6f08-9e63-4f70aee9d3b5@gmail.com

[PATCH] Re: Adding target and custom id links doesn't ask for description

[Ihor Radchenko]

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

Carlos Pita <carlosjosepita@gmail.com> writes:

> When storing a <<target>> or CUSTOM_ID link with org-store-link and then adding it with C-c C-l, org doesn’t ask for a description but automatically inserts an ugly description like:
>
>     ~/Desktop/Org/captures.org::target
>
> or:
>
>     ~/Desktop/Org/captures.org::#7EFBCEA1-0D17-45B9-A125-10FE581752F8
>
> The entire path is inserted even when the target itself is simply “target”. But, more importantly, no chance to change the description is given to the user.

I can replicate. I do see much point copy-pasting the link into
description. Moreover, this code is carried around since 15 years ago
from the commit

93c665b56 Intermediate state, I am just trying comiting now

:3

I suggest to set description to nil and thus ask the user in such
scenario.

See the attached patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-store-link-Default-to-empty-description-for-targ.patch --]
[-- Type: text/x-patch, Size: 1806 bytes --]

From 4a2079f237f5d23bfa83c2481f34d3bff27dd3e8 Mon Sep 17 00:00:00 2001
Message-Id: <4a2079f237f5d23bfa83c2481f34d3bff27dd3e8.1660109574.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Wed, 10 Aug 2022 13:25:26 +0800
Subject: [PATCH] org-store-link: Default to empty description for
 target/custom-id links

* lisp/ol.el (org-store-link): Use empty description by default (ask user).

Fixes https://orgmode.org/list/D99A712C-18D1-4A4F-8093-35A0BFB469C4@gmail.com
---
 lisp/ol.el | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index b7d74d0bf..29e0d3319 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -1577,10 +1577,8 @@ (defun org-store-link (arg &optional interactive?)
 		  t))))
 	(setq link (plist-get org-store-link-plist :link))
         ;; If store function actually set `:description' property, use
-        ;; it, even if it is nil.  Otherwise, fallback to link value.
-	(setq desc (if (plist-member org-store-link-plist :description)
-                       (plist-get org-store-link-plist :description)
-		     link)))
+        ;; it, even if it is nil.  Otherwise, fallback to nil (ask user).
+	(setq desc (plist-get org-store-link-plist :description)))
 
        ;; Store a link from a remote editing buffer.
        ((org-src-edit-buffer-p)
@@ -1697,9 +1695,7 @@ (defun org-store-link (arg &optional interactive?)
 		    (and org-id-link-to-org-use-id (org-entry-get nil "ID"))))
 	   (setq link (condition-case nil
 			  (prog1 (org-id-store-link)
-			    (setq desc (or (plist-get org-store-link-plist
-						      :description)
-					   "")))
+			    (setq desc (plist-get org-store-link-plist :description)))
 			(error
 			 ;; Probably before first headline, link only to file.
 			 (concat "file:"
-- 
2.35.1


[-- Attachment #3: Type: text/plain, Size: 207 bytes --]



-- 
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] Re: Adding target and custom id links doesn't ask for description

[Carlos Pita]

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

>
>
> I suggest to set description to nil and thus ask the user in such
> scenario.
>


I'm fine with that. Alternatively the default may be the target or custom
id (that is, the same that is in the link part), again letting the user
change it. Thanks!

Best regards,
Carlos

[-- Attachment #2: Type: text/html, Size: 940 bytes --]

Re: [PATCH] Re: Adding target and custom id links doesn't ask for description

[Max Nikulin]

On 10/08/2022 19:30, Carlos Pita wrote:
> 
>     I suggest to set description to nil and thus ask the user in such
>     scenario.
> 
> I'm fine with that. Alternatively the default may be the target or 
> custom id (that is, the same that is in the link part), again letting 
> the user change it. Thanks!

Carlos, have you tried Ihor's patch? I like the intention, but I do not 
see any effect. If I read the code correctly, first chunk is for C-u 
C-u, second one for id, not CUSTOM_ID links. The latter works on the 
main branch: I am prompted to confirm heading title as description. So I 
am confused.

In my environment my patches:

Max Nikulin. Re: Bug: org-store-link uses CUSTOM_ID instead of target 
point. Sat, 6 Nov 2021 19:51:29 +0700. 
https://list.orgmode.org/e2c807a7-1924-6f08-9e63-4f70aee9d3b5@gmail.com

helps for #CUSTOM_ID and <<target>> links. Heading title is offered in 
the first case and empty description in the latter.

P.S. The reason why file name is originally stored is that the link may 
be inserted to another file. When link is inserted to the same document, 
only target part is transformed to strip file name, description remains 
as it was stored.

Re: [PATCH] Re: Adding target and custom id links doesn't ask for description

[Carlos Pita]

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

>
>
> Carlos, have you tried Ihor's patch? I like the intention, but I do not
> see any effect.
>

Yes, I've tested it with target and custom_id links and, as you said, there
is no change in behavior. The entire URL is still pasted and no chance to
edit it is given to the user.

Best regards
--
Carlos

[-- Attachment #2: Type: text/html, Size: 808 bytes --]

[PATCH v2] Re: Adding target and custom id links doesn't ask for description

[Ihor Radchenko]

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

Carlos Pita <carlosjosepita@gmail.com> writes:

>> Carlos, have you tried Ihor's patch? I like the intention, but I do not
>> see any effect.
>>
>
> Yes, I've tested it with target and custom_id links and, as you said, there
> is no change in behavior. The entire URL is still pasted and no chance to
> edit it is given to the user.

Oops. Somehow some way things worked for me at some point when I was
making the patch.

See the updated version of the patch attached. It works on my side.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-org-store-link-Default-to-empty-description-for-t.patch --]
[-- Type: text/x-patch, Size: 2232 bytes --]

From 64c85eed9471b73e379e063dc3becefceb1ec650 Mon Sep 17 00:00:00 2001
Message-Id: <64c85eed9471b73e379e063dc3becefceb1ec650.1660366803.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Wed, 10 Aug 2022 13:25:26 +0800
Subject: [PATCH v2] org-store-link: Default to empty description for
 target/custom-id links

* lisp/ol.el (org-store-link): Use empty description by default (ask user).

Fixes https://orgmode.org/list/D99A712C-18D1-4A4F-8093-35A0BFB469C4@gmail.com
---
 lisp/ol.el | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index b7d74d0bf..358a96aae 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -1577,10 +1577,8 @@ (defun org-store-link (arg &optional interactive?)
 		  t))))
 	(setq link (plist-get org-store-link-plist :link))
         ;; If store function actually set `:description' property, use
-        ;; it, even if it is nil.  Otherwise, fallback to link value.
-	(setq desc (if (plist-member org-store-link-plist :description)
-                       (plist-get org-store-link-plist :description)
-		     link)))
+        ;; it, even if it is nil.  Otherwise, fallback to nil (ask user).
+	(setq desc (plist-get org-store-link-plist :description)))
 
        ;; Store a link from a remote editing buffer.
        ((org-src-edit-buffer-p)
@@ -1697,9 +1695,7 @@ (defun org-store-link (arg &optional interactive?)
 		    (and org-id-link-to-org-use-id (org-entry-get nil "ID"))))
 	   (setq link (condition-case nil
 			  (prog1 (org-id-store-link)
-			    (setq desc (or (plist-get org-store-link-plist
-						      :description)
-					   "")))
+			    (setq desc (plist-get org-store-link-plist :description)))
 			(error
 			 ;; Probably before first headline, link only to file.
 			 (concat "file:"
@@ -1761,8 +1757,7 @@ (defun org-store-link (arg &optional interactive?)
 
       ;; We're done setting link and desc, clean up
       (when (consp link) (setq cpltxt (car link) link (cdr link)))
-      (setq link (or link cpltxt)
-	    desc (or desc cpltxt))
+      (setq link (or link cpltxt))
       (cond ((not desc))
 	    ((equal desc "NONE") (setq desc nil))
 	    (t (setq desc (org-link-display-format desc))))
-- 
2.35.1


[-- Attachment #3: Type: text/plain, Size: 207 bytes --]



-- 
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 v2] Re: Adding target and custom id links doesn't ask for description

[Max Nikulin]

On 13/08/2022 12:01, Ihor Radchenko wrote:
> See the updated version of the patch attached. It works on my side.
> 
> --- a/lisp/ol.el
> +++ b/lisp/ol.el
> @@ -1577,10 +1577,8 @@ (defun org-store-link (arg &optional interactive?)
>  		  t))))
>  	(setq link (plist-get org-store-link-plist :link))
>          ;; If store function actually set `:description' property, use
> -        ;; it, even if it is nil.  Otherwise, fallback to link value.
> -	(setq desc (if (plist-member org-store-link-plist :description)
> -                       (plist-get org-store-link-plist :description)
> -		     link)))
> +        ;; it, even if it is nil.  Otherwise, fallback to nil (ask user).
> +	(setq desc (plist-get org-store-link-plist :description)))

I can not say that I understand design of `org-store-link` and 
`org-insert-link' pair, but I suspect that you might try to fix the 
issue in a wrong place.

It seems the same :link and :description value were set for purpose, see 
`org-insert-link' code:

	    (dolist (l org-stored-links)
	      (when (equal link (cadr l))
		(setq link (car l))
		(setq auto-desc t)))

However I am puzzled by

     (unless auto-desc

I would say that namely the same :link and :description should cause 
query to default description and user prompt.

I may be wrong. Maybe the protocol of communication between these 
functions may be documented with more details.

Re: [PATCH v2] Re: Adding target and custom id links doesn't ask for description

[Max Nikulin]

On 14/08/2022 16:53, Max Nikulin wrote:
> On 13/08/2022 12:01, Ihor Radchenko wrote:
>> See the updated version of the patch attached. It works on my side.
>>
>> --- a/lisp/ol.el
>> +++ b/lisp/ol.el
>> @@ -1577,10 +1577,8 @@ (defun org-store-link (arg &optional interactive?)
>>            t))))
>>      (setq link (plist-get org-store-link-plist :link))
>>          ;; If store function actually set `:description' property, use
>> -        ;; it, even if it is nil.  Otherwise, fallback to link value.
>> -    (setq desc (if (plist-member org-store-link-plist :description)
>> -                       (plist-get org-store-link-plist :description)
>> -             link)))
>> +        ;; it, even if it is nil.  Otherwise, fallback to nil (ask 
>> user).
>> +    (setq desc (plist-get org-store-link-plist :description)))
> 
> I can not say that I understand design of `org-store-link` and 
> `org-insert-link' pair, but I suspect that you might try to fix the 
> issue in a wrong place.
> 
> It seems the same :link and :description value were set for purpose, see 
> `org-insert-link' code:
> 
>          (dolist (l org-stored-links)
>            (when (equal link (cadr l))
>          (setq link (car l))
>          (setq auto-desc t)))

Ihor, I beg you pardon. I was wrong writing that you are trying to fix 
the issue in a wrong place. I figured out that the purpose of this 
snippet is completion by link description, not detecting of the case of 
identical link target and description. You have just committed the patch 
from the following thread where I provided more details:
Max Nikulin. ido, org-insert-link, and completion based on link 
description. Tue, 6 Sep 2022 21:34:12 +0700. 
https://list.orgmode.org/tf7lp4$5ha$1@ciao.gmane.io

Now I believe that your change is an improvement. I do not see any real 
reason to store the same description as the target. 
"~/Desktop/Org/captures.org::target" for <<target>> as default value of 
the description prompt might be a hint to the user for which link their 
should write description, but I hope, there is a better way to do the 
same. I have seen a mention that some export backend may create perhaps 
invisible links if description is omitted, but if it is still an issue, 
it should be fixed in affected backends.

Unfortunately, your patch does not fix the real issue. The title (not 
nil) should be saved for link description when the heading contains the 
CUSTOM_ID property. I tried to address it in the following patches:

Max Nikulin. Re: Bug: org-store-link uses CUSTOM_ID instead of target 
point. Sat, 6 Nov 2021 19:51:29 +0700. 
https://list.orgmode.org/e2c807a7-1924-6f08-9e63-4f70aee9d3b5@gmail.com

Re: [PATCH v2] Re: Adding target and custom id links doesn't ask for description

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> Unfortunately, your patch does not fix the real issue. The title (not 
> nil) should be saved for link description when the heading contains the 
> CUSTOM_ID property. I tried to address it in the following patches:
>
> Max Nikulin. Re: Bug: org-store-link uses CUSTOM_ID instead of target 
> point. Sat, 6 Nov 2021 19:51:29 +0700. 
> https://list.orgmode.org/e2c807a7-1924-6f08-9e63-4f70aee9d3b5@gmail.com

Does it mean that my patch + the patch you linked can be used to fix the
issue reported in this thread?

-- 
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 v2] Re: Adding target and custom id links doesn't ask for description

[Max Nikulin]

On 12/09/2022 18:02, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>> Unfortunately, your patch does not fix the real issue. The title (not
>> nil) should be saved for link description when the heading contains the
>> CUSTOM_ID property. I tried to address it in the following patches:
>>
>> Max Nikulin. Re: Bug: org-store-link uses CUSTOM_ID instead of target
>> point. Sat, 6 Nov 2021 19:51:29 +0700.
>> https://list.orgmode.org/e2c807a7-1924-6f08-9e63-4f70aee9d3b5@gmail.com
> 
> Does it mean that my patch + the patch you linked can be used to fix the
> issue reported in this thread?

To fix the #custom_id issue it is enough to commit 2 patches from the 
linked thread: reverting the change caused duplication of stored 
#custom_id links and an alternative fix to store link to <<target>>.

As to <<target>> links, I am unsure what description should be stored 
for <<target>>. I do not think it should be heading title and I do not 
like /full/path/file.org::target that looks strange when file is 
stripped for a link to the same file, but description contains full 
path. I proposed (3rd patch) to store just "target" as description, you 
may prefer nil. Both approaches allows to bypass broken auto-desc logic 
(which I would like to remove) and to show the prompt for description.

I consider your patch as a general improvement because it allows to 
avoid description = link in more cases.

Re: [PATCH v2] Re: Adding target and custom id links doesn't ask for description

[Ihor Radchenko]

Ihor Radchenko <yantar92@gmail.com> writes:

>> Yes, I've tested it with target and custom_id links and, as you said, there
>> is no change in behavior. The entire URL is still pasted and no chance to
>> edit it is given to the user.
>
> Oops. Somehow some way things worked for me at some point when I was
> making the patch.
>
> See the updated version of the patch attached. It works on my side.

According to the further discussion, this patch does not resolve the
original problem. However, it improves similar cases with link
descriptions being set equal to the link values.

Applied.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4fc2c8dd89bfbe9f6ad7620c1b4d6def4114489b

The original problem is resolved in another applied patch from an
earlier discussion.
https://list.orgmode.org/e2c807a7-1924-6f08-9e63-4f70aee9d3b5@gmail.com

-- 
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