emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Max Nikulin <manikulin@gmail.com>
To: Bastien <bzg@gnu.org>, Fr Ml <fr_ml@t-online.de>
Cc: emacs-orgmode@gnu.org
Subject: Re: Bug: org-store-link uses CUSTOM_ID instead of target point [9.4.4 (release_9.4.4 @ /usr/share/emacs/27.2/lisp/org/)]
Date: Sat, 6 Nov 2021 19:51:29 +0700	[thread overview]
Message-ID: <e2c807a7-1924-6f08-9e63-4f70aee9d3b5@gmail.com> (raw)
In-Reply-To: <87im3wqbk6.fsf@gnu.org>

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

On 06/05/2021 19:41, Bastien wrote:
> Fr Ml writes:
> 
>> I have a problem with the function org-store-link it doesn't work as
>> described in the documentation:
>> https://orgmode.org/manual/Handling-Links.html
>> "For Org files, if there is a '<<target>>' at point, the link points
>> to
>> the target."
> 
> Fixed in maint, thanks a lot for reporting this and Ihor for
> confirming the bug.

Bastien, unfortunately your fix caused duplication of stored links like 
"file:~/org/file.org::#custom_id" when point is outside of <<target>>. 
Earlier #CUSTOM_ID link was stored in addition to 
"file:~/org/file.org::*Heading" search link. My suggestion is to revert 
your patch and to just reset custom-id variable when <<target>> link is 
stored. Another effect or your patch, that I consider unintentional, is 
storing 
[[file:~/org/file.org::#custom_id][file:~/org/file.org::#custom_id]] 
instead of [[file:~/org/file.org::#custom_id][Heading]]. I prefer 
"original" behavior.

Third patch is intended to avoid links inserted as 
[[target][file:~/org/file.org::target]] in the case of same file. I 
suppose, just [[target]] is better. Current variant looks unbalanced and 
misleading. Of course, you are free to skip last patch.

I am confused by `org-insert-link' behavior. It inserts links like 
[[file:~/org/file.org::#custom_id][file:~/org/file.org::#custom_id]] 
without user prompt for description but 
[[file:~/org/file.org::#custom_id][Heading]] requires to confirm 
"Heading" description explicitly. I would expect that "raw" link is 
subject to ask user for more friendly option.


[-- Attachment #2: 0001-Revert-lisp-ol.el-Fix-bug-when-storing-links.patch --]
[-- Type: text/x-patch, Size: 2114 bytes --]

From 28f1d331888ebd22d60343bb06d3b307aff9fc93 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Fri, 5 Nov 2021 19:03:59 +0700
Subject: [PATCH 1/3] Revert "lisp/ol.el: Fix bug when storing links"

This reverts commit b4b35fc92d6ea8eb2ac061b8ccf026e9b4ebfe33.

Avoid duplication of CUSTOM_ID links.
---
 lisp/ol.el | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index aa1849715..6fe50ed60 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -1613,8 +1613,9 @@ non-nil."
 
        ((and (buffer-file-name (buffer-base-buffer)) (derived-mode-p 'org-mode))
 	(org-with-limited-levels
-         (cond
-	  ;; Store a link using the target at point.
+	 (setq custom-id (org-entry-get nil "CUSTOM_ID"))
+	 (cond
+	  ;; Store a link using the target at point
 	  ((org-in-regexp "[^<]<<\\([^<>]+\\)>>[^>]" 1)
 	   (setq cpltxt
 		 (concat "file:"
@@ -1622,15 +1623,6 @@ non-nil."
 			  (buffer-file-name (buffer-base-buffer)))
 			 "::" (match-string 1))
 		 link cpltxt))
-          ;; Store a link using the CUSTOM_ID property.
-          ((setq custom-id (org-entry-get nil "CUSTOM_ID"))
-           (setq cpltxt
-		 (concat "file:"
-			 (abbreviate-file-name
-			  (buffer-file-name (buffer-base-buffer)))
-			 "::#" custom-id)
-		 link cpltxt))
-          ;; Store a link using (and perhaps creating) the ID property.
 	  ((and (featurep 'org-id)
 		(or (eq org-id-link-to-org-use-id t)
 		    (and interactive?
@@ -1639,13 +1631,14 @@ non-nil."
 				      'create-if-interactive-and-no-custom-id)
 				  (not custom-id))))
 		    (and org-id-link-to-org-use-id (org-entry-get nil "ID"))))
+	   ;; Store a link using the ID at point
 	   (setq link (condition-case nil
 			  (prog1 (org-id-store-link)
 			    (setq desc (or (plist-get org-store-link-plist
 						      :description)
 					   "")))
 			(error
-			 ;; Probably before first headline, link only to file.
+			 ;; Probably before first headline, link only to file
 			 (concat "file:"
 				 (abbreviate-file-name
 				  (buffer-file-name (buffer-base-buffer))))))))
-- 
2.25.1


[-- Attachment #3: 0002-ol.el-Skip-CUSTOM_ID-when-target-link-is-stored.patch --]
[-- Type: text/x-patch, Size: 1225 bytes --]

From 1fb5a5e071141e47d551202eeca5776185ca8461 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Fri, 5 Nov 2021 19:49:23 +0700
Subject: [PATCH 2/3] ol.el: Skip #CUSTOM_ID when <<target>> link is stored

* list/ol.el (org-store-link): Suppress storing of
"file:file.org::#custom_id" link when point is <<target>>.

CUSTOM_ID link is stored as additional option, so new chunk of code
introduced by b4b35fc92 is not necessary. It prevented adding of
"file:file.org::*Heading" link and caused duplication of
"file:file.org::#custom_id" link.

Reported-by: Fr Ml <fr_ml@t-online.de>
Link: https://orgmode.org/list/aadb23f3-c0fe-19aa-be79-50e51d16c41a@t-online.de/
---
 lisp/ol.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index 6fe50ed60..5e1f1f2d2 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -1622,7 +1622,9 @@ non-nil."
 			 (abbreviate-file-name
 			  (buffer-file-name (buffer-base-buffer)))
 			 "::" (match-string 1))
-		 link cpltxt))
+		 link cpltxt
+                 ;; Do not append #CUSTOM_ID link below.
+                 custom-id nil))
 	  ((and (featurep 'org-id)
 		(or (eq org-id-link-to-org-use-id t)
 		    (and interactive?
-- 
2.25.1


[-- Attachment #4: 0003-ol.el-Avoid-links-like-target-file-org-test.org-targ.patch --]
[-- Type: text/x-patch, Size: 1666 bytes --]

From f307c7059744b28b26f5c3b8dd4a0f563b22e586 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Sat, 6 Nov 2021 18:23:25 +0700
Subject: [PATCH 3/3] ol.el: Avoid links like
 "[[target][file:~/org/test.org::target]]"

* lisp/ol.el (org-store-link): Do not set description for "<<target>>"
links to avoid case when it is more detailed than link target.

While inserting to the same file, file part of the link target is
stripped, description is inserted without modification.  I do not think,
file path adds real value in comparison to "[[target]]" link to some
point in the same file.  A side effect is user prompt for description
since link and description are not identical any more.
---
 lisp/ol.el | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index 5e1f1f2d2..ec5427e7c 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -1617,12 +1617,17 @@ non-nil."
 	 (cond
 	  ;; Store a link using the target at point
 	  ((org-in-regexp "[^<]<<\\([^<>]+\\)>>[^>]" 1)
-	   (setq cpltxt
+	   (setq link
 		 (concat "file:"
 			 (abbreviate-file-name
 			  (buffer-file-name (buffer-base-buffer)))
 			 "::" (match-string 1))
-		 link cpltxt
+                 ;; Target may be shortened when link is inserted.
+                 ;; Avoid [[target][file:~/org/test.org::target]]
+                 ;; links.  Maybe the case of identical target and
+                 ;; description should be handled by `org-insert-link'.
+                 cpltxt nil
+                 desc nil
                  ;; Do not append #CUSTOM_ID link below.
                  custom-id nil))
 	  ((and (featurep 'org-id)
-- 
2.25.1


  reply	other threads:[~2021-11-06 12:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-04 19:17 Fr Ml
2021-05-05  2:34 ` Ihor Radchenko
2021-05-06 12:41 ` Bastien
2021-11-06 12:51   ` Max Nikulin [this message]
2021-05-06 20:08 ` Table alignment problem Fr Ml
2021-05-07 14:50   ` Ihor Radchenko
2021-05-08  6:37     ` Jeremie Juste
2021-05-08  7:45       ` Fr Ml
2021-05-08  7:48     ` Fr Ml
2021-05-08  8:24       ` Ihor Radchenko
     [not found]         ` <5ebfdc6f-6e40-1470-4379-4a5a2b666aa7@t-online.de>
     [not found]           ` <87czu1pivb.fsf@localhost>
     [not found]             ` <6c4c8084-3800-c085-b724-f5653d7c20bd@t-online.de>
     [not found]               ` <87eeeh17fq.fsf@localhost>
     [not found]                 ` <8975eded-5a22-cb9c-b85e-f3b523f16411@t-online.de>
     [not found]                   ` <871ragff66.fsf@localhost>
     [not found]                     ` <84936763-9384-7e6c-5304-b94217298b9a@t-online.de>
     [not found]                       ` <87lf8o9pgn.fsf@localhost>
     [not found]                         ` <eb465a73-691d-01be-135e-f58a9ff36173@t-online.de>
     [not found]                           ` <87im3s9ie8.fsf@localhost>
     [not found]                             ` <2c2f2204-14b1-49b2-658b-2b8b75ab86b9@t-online.de>
2021-05-15  9:26                               ` [PATCH] Pixel-wise table alignment Ihor Radchenko

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=e2c807a7-1924-6f08-9e63-4f70aee9d3b5@gmail.com \
    --to=manikulin@gmail.com \
    --cc=bzg@gnu.org \
    --cc=emacs-orgmode@gnu.org \
    --cc=fr_ml@t-online.de \
    --subject='Re: Bug: org-store-link uses CUSTOM_ID instead of target point [9.4.4 (release_9.4.4 @ /usr/share/emacs/27.2/lisp/org/)]' \
    /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

Code repositories for project(s) associated with this 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).