* [PATCH] org-id: allow using parent's existing id in links to headlines
@ 2023-07-24 11:40 Rick Lupton
2023-07-25 7:43 ` Ihor Radchenko
2023-11-04 23:01 ` Rick Lupton
0 siblings, 2 replies; 16+ messages in thread
From: Rick Lupton @ 2023-07-24 11:40 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]
Hi,
Here is a small new feature for org-id that I have been using and finding useful. The patch adds the option to look for ancestors of the current headline that have an ID defined and use that together with a link search string to link to specific headlines, without needing every single headline to have its own ID.
For example if you have:
#+begin_example
* H1
:PROPERTIES:
:ID: abc
:END:
** H2
<point>Link to here
#+end_example
with `org-id-link-to-org-use-id' set to `t`, the result of org-store-link will be that "H2" has a new id generated, and the link is to that new ID: `[[id:new-id][H2]]`.
Now, with `org-id-link-to-org-use-id' set to `inherit`, "H2" is not modified, and the resulting link is `[[id:abc::*H2][H2]]`, which will still take you to the same place as long as the sub-heading is unique within the parent heading with an ID.
As an example, I find this useful in situations like this:
#+begin_example
* Project 1
:PROPERTIES:
:ID: project-1
:END:
** <2023-07-01> Meeting A
** <2023-07-08> Meeting B
** <2023-07-15> Meeting C
#+end_example
... so that I can link to specific meetings without needing every one to have its own org ID.
Feedback on the patch welcome. If you would like to merge this I will (I assume) need to sort out FSF copyright assignment and update ORG-NEWS and the manual.
Best
Rick
[-- Attachment #2: 0001-lisp-org-id.el-Allow-using-a-parent-s-existing-id.patch --]
[-- Type: application/octet-stream, Size: 8328 bytes --]
From 99b439865b214ecfbbb2b6685ed7782293c157c1 Mon Sep 17 00:00:00 2001
From: Rick Lupton <mail@ricklupton.name>
Date: Mon, 24 Jul 2023 12:29:30 +0100
Subject: [PATCH] lisp/org-id.el: Allow using a parent's existing id
* lisp/ol.el (org-store-link): When `org-id-link-to-org-use-id` is
`inherit`, look for existing IDs on ancestors of the current headline,
and use a link search string to find the current headline within that
ancestor.
* lisp/org-id.el (org-id-link-to-org-use-id): Introduce new `inherit`
value.
(org-id-get-create, org-id-get, org-id-store-link): Add optional
`inherit` argument which considers parents' IDs if the current entry
does not have one.
* testing/lisp/test-ol.el: Add test for `org-id-link-to-org-use-id` set
to `inherit`.
This feature allows for more precise links when using org-id to link to
org headings, without requiring every single headline to have an id.
---
lisp/ol.el | 38 +++++++++++++++++++++++++++++++++++++-
lisp/org-id.el | 27 +++++++++++++++++++--------
testing/lisp/test-ol.el | 20 ++++++++++++++++++++
3 files changed, 76 insertions(+), 9 deletions(-)
diff --git a/lisp/ol.el b/lisp/ol.el
index 3a8ca5f39..2e863e47b 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -63,7 +63,7 @@
(declare-function org-find-property "org" (property &optional value))
(declare-function org-get-heading "org" (&optional no-tags no-todo no-priority no-comment))
(declare-function org-id-find-id-file "org-id" (id))
-(declare-function org-id-store-link "org-id" ())
+(declare-function org-id-store-link "org-id" (&optional inherit))
(declare-function org-insert-heading "org" (&optional arg invisible-ok top))
(declare-function org-load-modules-maybe "org" (&optional force))
(declare-function org-mark-ring-push "org" (&optional pos buffer))
@@ -1700,6 +1700,42 @@ non-nil."
(concat "file:"
(abbreviate-file-name
(buffer-file-name (buffer-base-buffer))))))))
+ ((and (featurep 'org-id)
+ (eq org-id-link-to-org-use-id 'inherit))
+ ;; Store a link using the inherited ID and search string
+ (setq cpltxt (condition-case nil
+ (prog1 (org-id-store-link 'inherit)
+ (setq desc (plist-get org-store-link-plist :description)))
+ (error
+ ;; Probably before first headline, link only to file
+ (concat "file:"
+ (abbreviate-file-name
+ (buffer-file-name (buffer-base-buffer)))))))
+ ;; Add a context search string, limited by current region
+ (when (org-xor org-link-context-for-files (equal arg '(4)))
+ (let* ((element (org-element-at-point))
+ (name (org-element-property :name element))
+ (context
+ (cond
+ ((let ((region (org-link--context-from-region)))
+ (and region (org-link--normalize-string region t))))
+ (name)
+ ((org-before-first-heading-p)
+ (org-link--normalize-string (org-current-line-string) t))
+ (t (org-link-heading-search-string)))))
+ (when (org-string-nw-p context)
+ (setq cpltxt (format "%s::%s" cpltxt context))
+ (setq desc
+ (or name
+ ;; Although description is not a search
+ ;; string, use `org-link--normalize-string'
+ ;; to prettify it (contiguous white spaces)
+ ;; and remove volatile contents (statistics
+ ;; cookies).
+ (and (not (org-before-first-heading-p))
+ (org-link--normalize-string
+ (org-get-heading t t t t)))
+ "NONE"))))))
(t
;; Just link to current headline.
(setq cpltxt (concat "file:"
diff --git a/lisp/org-id.el b/lisp/org-id.el
index dae3a0ca8..7b57c8289 100644
--- a/lisp/org-id.el
+++ b/lisp/org-id.el
@@ -114,6 +114,10 @@ create-if-interactive-and-no-custom-id
use-existing
Use existing ID, do not create one.
+inherit
+ Use existing ID from a parent headline, and use a text
+ search to find this headline within it.
+
nil Never use an ID to make a link, instead link using a text search for
the headline text."
:group 'org-link-store
@@ -255,14 +259,17 @@ This variable is only relevant when `org-id-track-globally' is set."
;;; The API functions
;;;###autoload
-(defun org-id-get-create (&optional force)
+(defun org-id-get-create (&optional force inherit)
"Create an ID for the current entry and return it.
If the entry already has an ID, just return it.
-With optional argument FORCE, force the creation of a new ID."
+With optional argument FORCE, force the creation of a new ID.
+With optional argument INHERIT, consider parents' IDs if the
+current entry does not have one."
(interactive "P")
(when force
- (org-entry-put (point) "ID" nil))
- (org-id-get (point) 'create))
+ (org-entry-put (point) "ID" nil)
+ (setq inherit nil))
+ (org-id-get (point) 'create nil inherit))
;;;###autoload
(defun org-id-copy ()
@@ -277,15 +284,16 @@ This is useful when working with contents in a temporary buffer
that will be copied back to the original.")
;;;###autoload
-(defun org-id-get (&optional epom create prefix)
+(defun org-id-get (&optional epom create prefix inherit)
"Get the ID property of the entry at EPOM.
EPOM is an element, marker, or buffer position.
If EPOM is nil, refer to the entry at point.
If the entry does not have an ID, the function returns nil.
+If INHERIT is non-nil, parents' IDs are also considered.
However, when CREATE is non-nil, create an ID if none is present already.
PREFIX will be passed through to `org-id-new'.
In any case, the ID of the entry is returned."
- (let ((id (org-entry-get epom "ID")))
+ (let ((id (org-entry-get epom "ID" inherit)))
(cond
((and id (stringp id) (string-match "\\S-" id))
id)
@@ -680,14 +688,17 @@ optional argument MARKERP, return the position as a new marker."
;; so we do have to add it to `org-store-link-functions'.
;;;###autoload
-(defun org-id-store-link ()
+(defun org-id-store-link (&optional inherit)
"Store a link to the current entry, using its ID.
+If INHERIT is non-nil, consider also parents' IDs if the current
+entry does not have an ID.
+
If before first heading store first title-keyword as description
or filename if no title."
(interactive)
(when (and (buffer-file-name (buffer-base-buffer)) (derived-mode-p 'org-mode))
- (let* ((link (concat "id:" (org-id-get-create)))
+ (let* ((link (concat "id:" (org-id-get-create nil inherit)))
(case-fold-search nil)
(desc (save-excursion
(org-back-to-heading-or-point-min t)
diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el
index a38d9f979..7a4d9999a 100644
--- a/testing/lisp/test-ol.el
+++ b/testing/lisp/test-ol.el
@@ -381,6 +381,26 @@ See https://github.com/yantar92/org/issues/4."
(equal (format "[[file:%s::*foo bar][foo bar]]" file file)
(org-store-link nil)))))))
+(ert-deftest test-org-link/store-link-with-id ()
+ "Test `org-store-link' specifications with org-id."
+ ;; On a headline, link to that headline's ID. Use heading as the
+ ;; description of the link.
+ (should
+ (let ((org-stored-links nil)
+ (org-id-link-to-org-use-id t))
+ (org-test-with-temp-text-in-file "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n"
+ (equal "[[id:abc][H1]]"
+ (org-store-link nil)))))
+ ;; On a headline without an ID, link to that headline's parent's ID,
+ ;; with the current headline as context. Use heading as the
+ ;; description of the link.
+ (should
+ (let ((org-stored-links nil)
+ (org-id-link-to-org-use-id 'inherit))
+ (org-test-with-temp-text-in-file "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n** H2\n** H3<point>\n** H4\n"
+ (let ((link (org-store-link nil)))
+ (equal link "[[id:abc::*H3][H3]]"))))))
+
\f
;;; Radio Targets
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-07-24 11:40 [PATCH] org-id: allow using parent's existing id in links to headlines Rick Lupton
@ 2023-07-25 7:43 ` Ihor Radchenko
2023-07-25 15:16 ` Max Nikulin
2023-11-09 20:56 ` Rick Lupton
2023-11-04 23:01 ` Rick Lupton
1 sibling, 2 replies; 16+ messages in thread
From: Ihor Radchenko @ 2023-07-25 7:43 UTC (permalink / raw)
To: Rick Lupton; +Cc: emacs-orgmode
"Rick Lupton" <mail@ricklupton.name> writes:
> Here is a small new feature for org-id that I have been using and finding useful. The patch adds the option to look for ancestors of the current headline that have an ID defined and use that together with a link search string to link to specific headlines, without needing every single headline to have its own ID.
I think that it will be a reasonable addition.
> Now, with `org-id-link-to-org-use-id' set to `inherit`, "H2" is not modified, and the resulting link is `[[id:abc::*H2][H2]]`, which will still take you to the same place as long as the sub-heading is unique within the parent heading with an ID.
What about inherited CUSTOM_ID?
> Feedback on the patch welcome. If you would like to merge this I will (I assume) need to sort out FSF copyright assignment and update ORG-NEWS and the manual.
Yes, on both questions.
See https://orgmode.org/worg/org-contribute.html#copyright
> + ((and (featurep 'org-id)
> + (eq org-id-link-to-org-use-id 'inherit))
What if none of the parents have an ID?
> + ;; Store a link using the inherited ID and search string
> + (setq cpltxt (condition-case nil
> + (prog1 (org-id-store-link 'inherit)
> ...
> + "NONE"))))))
This is code duplication from another branch of the same function.
Please, rewrite without copy-pasting large chunks of code.
For example, you can extract the common parts of the code into a private
helper function.
> ;;;###autoload
> -(defun org-id-store-link ()
> +(defun org-id-store-link (&optional inherit)
> "Store a link to the current entry, using its ID.
>
> +If INHERIT is non-nil, consider also parents' IDs if the current
> +entry does not have an ID.
> +
This will no longer store a link to current entry with INHERIT argument.
The search string will be missing from the link path.
Ideally, we should have all the necessary logic to store the link within
`org-id-store-link' and then use `org-link-set-parameters' to configure
id links.
--
Ihor Radchenko // yantar92,
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-07-25 7:43 ` Ihor Radchenko
@ 2023-07-25 15:16 ` Max Nikulin
2023-07-26 8:10 ` Ihor Radchenko
2023-11-09 20:56 ` Rick Lupton
1 sibling, 1 reply; 16+ messages in thread
From: Max Nikulin @ 2023-07-25 15:16 UTC (permalink / raw)
To: emacs-orgmode; +Cc: Rick Lupton
On 25/07/2023 14:43, Ihor Radchenko wrote:
> "Rick Lupton" writes:
> >> Now, with `org-id-link-to-org-use-id' set to `inherit`, "H2" is not
>> modified, and the resulting link is `[[id:abc::*H2][H2]]`, which will
>> still take you to the same place as long as the sub-heading is unique
>> within the parent heading with an ID.
> What about inherited CUSTOM_ID?
I am not excited by the idea of extending id links for heading
hierarchy. From my point of view it is more natural to add the ID
property to the heading that should be link target.
Sometimes I do not mind to disambiguate heading search link by
specifying title of its ancestor. I usually add the CUSTOM_ID property
or rename heading to be unique.
I am afraid that allowing arbitrary link types to specify path to an
element is overkill. It is not XPath and not CSS selectors.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-07-25 15:16 ` Max Nikulin
@ 2023-07-26 8:10 ` Ihor Radchenko
2023-07-27 0:16 ` Samuel Wales
2023-07-28 19:56 ` [PATCH] org-id: allow using parent's existing id in links to headlines Rick Lupton
0 siblings, 2 replies; 16+ messages in thread
From: Ihor Radchenko @ 2023-07-26 8:10 UTC (permalink / raw)
To: Max Nikulin; +Cc: emacs-orgmode, Rick Lupton
Max Nikulin <manikulin@gmail.com> writes:
> I am not excited by the idea of extending id links for heading
> hierarchy. From my point of view it is more natural to add the ID
> property to the heading that should be link target.
>
> Sometimes I do not mind to disambiguate heading search link by
> specifying title of its ancestor. I usually add the CUSTOM_ID property
> or rename heading to be unique.
>
> I am afraid that allowing arbitrary link types to specify path to an
> element is overkill. It is not XPath and not CSS selectors.
I am looking at it from an opposite direction: we already have file:
links with ::search term, but file is not a very reliable link anchor.
File ID will persist even when the file is moved. So, instead of having
something like <file:/path/to/foo.org::* Heading>, we should better also
provide <id:ID::*heading> with ID defined in the top-level property
drawer. ID being some sub-heading is then a natural extension of the
same idea.
--
Ihor Radchenko // yantar92,
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-07-26 8:10 ` Ihor Radchenko
@ 2023-07-27 0:16 ` Samuel Wales
2023-07-27 7:42 ` IDs below headline level (for paragraphs, lists, etc) (was: [PATCH] org-id: allow using parent's existing id in links to headlines) Ihor Radchenko
2023-07-28 19:56 ` [PATCH] org-id: allow using parent's existing id in links to headlines Rick Lupton
1 sibling, 1 reply; 16+ messages in thread
From: Samuel Wales @ 2023-07-27 0:16 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: Max Nikulin, emacs-orgmode, Rick Lupton
i can see the appeal given the granularity of id [headings, files]. yu
want to point to smaller things. but what if those smaller things
could have ids without drawers? id markers. then changes in
surrounding text would not break anything.
On 7/26/23, Ihor Radchenko <yantar92@posteo.net> wrote:
> Max Nikulin <manikulin@gmail.com> writes:
>
>> I am not excited by the idea of extending id links for heading
>> hierarchy. From my point of view it is more natural to add the ID
>> property to the heading that should be link target.
>>
>> Sometimes I do not mind to disambiguate heading search link by
>> specifying title of its ancestor. I usually add the CUSTOM_ID property
>> or rename heading to be unique.
>>
>> I am afraid that allowing arbitrary link types to specify path to an
>> element is overkill. It is not XPath and not CSS selectors.
>
> I am looking at it from an opposite direction: we already have file:
> links with ::search term, but file is not a very reliable link anchor.
> File ID will persist even when the file is moved. So, instead of having
> something like <file:/path/to/foo.org::* Heading>, we should better also
> provide <id:ID::*heading> with ID defined in the top-level property
> drawer. ID being some sub-heading is then a natural extension of the
> same idea.
>
> --
> Ihor Radchenko // yantar92,
> 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>
>
>
--
The Kafka Pandemic
A blog about science, health, human rights, and misopathy:
https://thekafkapandemic.blogspot.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* IDs below headline level (for paragraphs, lists, etc) (was: [PATCH] org-id: allow using parent's existing id in links to headlines)
2023-07-27 0:16 ` Samuel Wales
@ 2023-07-27 7:42 ` Ihor Radchenko
2023-07-28 20:00 ` Rick Lupton
0 siblings, 1 reply; 16+ messages in thread
From: Ihor Radchenko @ 2023-07-27 7:42 UTC (permalink / raw)
To: Samuel Wales; +Cc: Max Nikulin, emacs-orgmode, Rick Lupton
Samuel Wales <samologist@gmail.com> writes:
> ... but what if those smaller things
> could have ids without drawers? id markers. then changes in
> surrounding text would not break anything.
I recall similar idea raised in
https://list.orgmode.org/orgmode/CAJniy+OVD0NCWZZTPit5T7wvsbLbgLLXZmPub5tgq3gsHsGhYw@mail.gmail.com/
But there was not much interest.
It was pointed that we already have link targets, although they are not
global. Making link targets global is doable.
--
Ihor Radchenko // yantar92,
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-07-26 8:10 ` Ihor Radchenko
2023-07-27 0:16 ` Samuel Wales
@ 2023-07-28 19:56 ` Rick Lupton
2023-07-29 8:33 ` Ihor Radchenko
1 sibling, 1 reply; 16+ messages in thread
From: Rick Lupton @ 2023-07-28 19:56 UTC (permalink / raw)
To: emacs-orgmode
Hi Ihor,
Thanks for the comments, I will take a look. A question below.
On Wed, 26 Jul 2023, at 9:10 AM, Ihor Radchenko wrote:
>
> I am looking at it from an opposite direction: we already have file:
> links with ::search term, but file is not a very reliable link anchor.
> File ID will persist even when the file is moved. So, instead of having
> something like <file:/path/to/foo.org::* Heading>, we should better also
> provide <id:ID::*heading> with ID defined in the top-level property
> drawer. ID being some sub-heading is then a natural extension of the
> same idea.
This is a good description of the motivation from my point of view.
> What about inherited CUSTOM_ID?
I’m not sure what you mean.
Are you thinking of CUSTOM_ID links, and whether they would behave consistently with a search string to this proposal? Like: [[custom-id:my-id::*H2][H2]]
Or using custom id as a search string? Like: [[id:abc::#my-id][Description]]
Thanks
Rick
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: IDs below headline level (for paragraphs, lists, etc) (was: [PATCH] org-id: allow using parent's existing id in links to headlines)
2023-07-27 7:42 ` IDs below headline level (for paragraphs, lists, etc) (was: [PATCH] org-id: allow using parent's existing id in links to headlines) Ihor Radchenko
@ 2023-07-28 20:00 ` Rick Lupton
0 siblings, 0 replies; 16+ messages in thread
From: Rick Lupton @ 2023-07-28 20:00 UTC (permalink / raw)
To: Ihor Radchenko, Samuel Wales; +Cc: Max Nikulin, Y. E.
I can see this being useful in general, but not avoiding the need for my patch. Org links using search strings already strike a good compromise between working with arbitrary plain text, and allowing links to specific locations. When a search string is enough to find the thing you want to link to, there’s no need to add more IDs manually.
If this is already intended to be an unrelated discussion then feel free to ignore this comment!
On Thu, 27 Jul 2023, at 8:42 AM, Ihor Radchenko wrote:
> Samuel Wales <samologist@gmail.com> writes:
>
>> ... but what if those smaller things
>> could have ids without drawers? id markers. then changes in
>> surrounding text would not break anything.
>
> I recall similar idea raised in
> https://list.orgmode.org/orgmode/CAJniy+OVD0NCWZZTPit5T7wvsbLbgLLXZmPub5tgq3gsHsGhYw@mail.gmail.com/
>
> But there was not much interest.
>
> It was pointed that we already have link targets, although they are not
> global. Making link targets global is doable.
>
> --
> Ihor Radchenko // yantar92,
> 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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-07-28 19:56 ` [PATCH] org-id: allow using parent's existing id in links to headlines Rick Lupton
@ 2023-07-29 8:33 ` Ihor Radchenko
0 siblings, 0 replies; 16+ messages in thread
From: Ihor Radchenko @ 2023-07-29 8:33 UTC (permalink / raw)
To: Rick Lupton; +Cc: emacs-orgmode
"Rick Lupton" <mail@ricklupton.name> writes:
>> What about inherited CUSTOM_ID?
>
> I’m not sure what you mean.
>
> Are you thinking of CUSTOM_ID links, and whether they would behave consistently with a search string to this proposal? Like: [[custom-id:my-id::*H2][H2]]
>
> Or using custom id as a search string? Like: [[id:abc::#my-id][Description]]
No. I was thinking about something like [[#my-id::search string]].
However, this will only make sense in internal links. file: links would
need multiple search terms, which we currently do not support.
So, never mind my comment.
--
Ihor Radchenko // yantar92,
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-07-24 11:40 [PATCH] org-id: allow using parent's existing id in links to headlines Rick Lupton
2023-07-25 7:43 ` Ihor Radchenko
@ 2023-11-04 23:01 ` Rick Lupton
2023-11-05 12:31 ` Ihor Radchenko
1 sibling, 1 reply; 16+ messages in thread
From: Rick Lupton @ 2023-11-04 23:01 UTC (permalink / raw)
To: Y. E.
I realised there is another question here about how search strings are used in org-id links.
Consider this example file:
* Heading
:PROPERTIES:
:ID: 06E767E6-6145-45EB-B736-D350449126EC
:END:
#+name: named-thing
#+begin_example
Hi!
#+end_example
By default (`org-id-link-to-org-use-id` is nil), with point on `#+name: named-thing`, calling `org-store-link` will give a link like `[[file:test.org::named-thing][named-thing]]` which leads directly to the named example block. Different uses can also lead to search strings which link to headings, selected text in the region, or the current line's text.
When `org-id-link-to-org-use-id` is non-nil, none of this happens -- calling `org-store-link` anywhere within the subtree will result in a link `[[id:06E767E6-6145-45EB-B736-D350449126EC][Heading]]` with no additional search string.
My previous patch changes the behaviour when `org-id-link-to-org-use-id` has a new value (`inherit`) in two ways:
(a) org-ids from parent headings are considered when choosing the ID to link to, and
(b) search strings are added to the link
But these are actually two independent things. So my question is: should search strings be added to all org-id links?
This would make org-id links more powerful/precise (because you can link to more precise locations within the subtree), and simplifies the code in `org-store-link` in my patch (because point [b] above would apply to all org-id links, not just the new 'inherit ones), but it could change the behaviour when calling `org-store-link` with an active region or when point is on a named element.
Depending on the answer, I can update the patch accordingly.
Thanks,
Rick
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-11-04 23:01 ` Rick Lupton
@ 2023-11-05 12:31 ` Ihor Radchenko
0 siblings, 0 replies; 16+ messages in thread
From: Ihor Radchenko @ 2023-11-05 12:31 UTC (permalink / raw)
To: Rick Lupton; +Cc: Y. E.
"Rick Lupton" <mail@ricklupton.name> writes:
> My previous patch changes the behaviour when `org-id-link-to-org-use-id` has a new value (`inherit`) in two ways:
>
> (a) org-ids from parent headings are considered when choosing the ID to link to, and
> (b) search strings are added to the link
>
> But these are actually two independent things. So my question is: should search strings be added to all org-id links?
> This would make org-id links more powerful/precise (because you can link to more precise locations within the subtree), and simplifies the code in `org-store-link` in my patch (because point [b] above would apply to all org-id links, not just the new 'inherit ones), but it could change the behaviour when calling `org-store-link` with an active region or when point is on a named element.
Sounds as a reasonable default, but users should have an option to
revert to previous behaviour with heading id being stored.
Otherwise, we may break the muscle memory for people used to what
happens now.
--
Ihor Radchenko // yantar92,
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-07-25 7:43 ` Ihor Radchenko
2023-07-25 15:16 ` Max Nikulin
@ 2023-11-09 20:56 ` Rick Lupton
2023-11-10 10:03 ` Ihor Radchenko
1 sibling, 1 reply; 16+ messages in thread
From: Rick Lupton @ 2023-11-09 20:56 UTC (permalink / raw)
To: Y. E.
On Tue, 25 Jul 2023, at 8:43 AM, Ihor Radchenko wrote:
> Ideally, we should have all the necessary logic to store the link within
> `org-id-store-link' and then use `org-link-set-parameters' to configure
> id links.
I agree this would be neater, but looking at how this would work, I have a question:
Behaviour in `org-store-link` currently depends on the `interactive?` argument, e.g. in this logic
(and interactive?
(or (eq org-id-link-to-org-use-id 'create-if-interactive)
(and (eq org-id-link-to-org-use-id
'create-if-interactive-and-no-custom-id)
(not custom-id))))
To move this logic to `org-id-store-link`, is there a way that `org-id-store-link` can tell whether `org-store-link` was called (a) interactively, or (b) with the `interactive?` argument true?
Thanks
Rick
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-11-09 20:56 ` Rick Lupton
@ 2023-11-10 10:03 ` Ihor Radchenko
2023-11-19 15:21 ` Rick Lupton
0 siblings, 1 reply; 16+ messages in thread
From: Ihor Radchenko @ 2023-11-10 10:03 UTC (permalink / raw)
To: Rick Lupton; +Cc: Y. E.
"Rick Lupton" <mail@ricklupton.name> writes:
> On Tue, 25 Jul 2023, at 8:43 AM, Ihor Radchenko wrote:
>> Ideally, we should have all the necessary logic to store the link within
>> `org-id-store-link' and then use `org-link-set-parameters' to configure
>> id links.
>
> I agree this would be neater, but looking at how this would work, I have a question:
>
> Behaviour in `org-store-link` currently depends on the `interactive?` argument, e.g. in this logic
>
> (and interactive?
> (or (eq org-id-link-to-org-use-id 'create-if-interactive)
> (and (eq org-id-link-to-org-use-id
> 'create-if-interactive-and-no-custom-id)
> (not custom-id))))
>
> To move this logic to `org-id-store-link`, is there a way that `org-id-store-link` can tell whether `org-store-link` was called (a) interactively, or (b) with the `interactive?` argument true?
I think that we need to make a change in the rules for :store functions.
`interactive?' may be passed as the argument to these functions.
In order to not cause breakage, we need something like
(condition-case nil
(funcall protocol path desc backend info)
;; XXX: The function used (< Org 9.4) to accept only
;; three mandatory arguments. Type-specific `:export'
;; functions in the wild may not handle current
;; signature. Provide backward compatibility support
;; for them.
(wrong-number-of-arguments
(funcall protocol path desc backend)))
to keep the old :store functions that accept 0 arguments working.
--
Ihor Radchenko // yantar92,
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-11-10 10:03 ` Ihor Radchenko
@ 2023-11-19 15:21 ` Rick Lupton
2023-12-04 13:23 ` Rick Lupton
2023-12-10 13:35 ` Ihor Radchenko
0 siblings, 2 replies; 16+ messages in thread
From: Rick Lupton @ 2023-11-19 15:21 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode list
[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]
Here's an updated patch, which adds (optional) search strings to ID links, and the option to inherit ID targets from parent headline / the top level file properties. I've also updated ORG-NEWS and the manual, and added tests.
I think I've fixed all the issues with my first patch about which headline gets used for the description when inheriting IDs, what happens if there is no ID, etc.
> Ideally, we should have all the necessary logic to store the link within `org-id-store-link' and then use `org-link-set-parameters' to configure id links.
> ...
> I think that we need to make a change in the rules for :store functions. `interactive?' may be passed as the argument to these functions.
I've also moved the org-id specific logic from `org-store-link` to `org-id-store-link`, and added the `interactive?` argument to link store functions as discussed.
>> So my question is: should search strings be added to all org-id links?
> Sounds as a reasonable default, but users should have an option to revert to previous behaviour with heading id being stored.
The default value for the new option `org-id-link-use-context` is `t`, but it can be set to `nil` (or disabled with a prefix argument to `org-store-link` temporarily). This is a change in default behaviour when storing ID links with point at a subheading, named block, or target, or with an active region.
The option `org-id-link-consider-parent-id` I've left with a default value of `nil`, since I'm not sure if everyone will want this behaviour.
Thanks
Rick
[-- Attachment #2: 0001-org-id.el-Extend-links-with-search-strings-inherit-p.patch --]
[-- Type: application/octet-stream, Size: 38990 bytes --]
From b94cd1beba9b54ae23947d59e6a5cdc77fb2640e Mon Sep 17 00:00:00 2001
From: Rick Lupton <mail@ricklupton.name>
Date: Sun, 19 Nov 2023 14:52:05 +0000
Subject: [PATCH] org-id.el: Extend links with search strings, inherit parent
IDs
* lisp/ol.el (org-store-link): Refactor org-id links to use standard
`org-store-link-functions'.
(org-link-precise-link-target): New function extracting logic to
identify a precise link target, e.g. a heading, named object, or text
search.
(org-link-try-link-store-functions): Extract logic to call external
link store functions. Pass them a new `interactive?' argument.
* lisp/org-id.el (org-id-link-consider-parent-id): New option to allow
a parent heading with an id to be considered as a link target.
(org-id-link-use-context): New option equivalent to
`org-link-context-for-files`, but for org-id links.
(org-id-get-create, org-id-get): Add optional `inherit' argument which
considers parents' IDs if the current entry does not have one.
(org-id-store-link): Move logic from `org-store-link' here to
determine when an org-id link should be stored. Consider IDs of parent
headings as link targets when current heading has no ID and
`org-id-link-consider-parent-id' is set. Add a search string to the
link when enabled.
(org-id-open): Recognise search strings after "::" in org-id links.
* testing/lisp/test-ol.el: Add tests for
`org-link-precise-link-target' and `org-id-store-link' functions,
testing new options.
* doc/org-manual.org: Update documentation about links.
* etc/ORG-NEWS: Document changes and new options.
These feature allows for more precise links when using org-id to link to
org headings, without requiring every single headline to have an id.
Link: https://list.orgmode.org/118435e8-0b20-46fd-af6a-88de8e19fac6@app.fastmail.com/
---
doc/org-manual.org | 118 +++++++++++++-----------
etc/ORG-NEWS | 33 +++++++
lisp/ol.el | 198 ++++++++++++++++++++++++----------------
lisp/org-id.el | 96 ++++++++++++++++---
testing/lisp/test-ol.el | 134 +++++++++++++++++++++++++++
5 files changed, 433 insertions(+), 146 deletions(-)
diff --git a/doc/org-manual.org b/doc/org-manual.org
index 85568e7ab..eedaf365a 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -3296,10 +3296,6 @@ Here is the full set of built-in link types:
File links. File name may be remote, absolute, or relative.
- Additionally, you can specify a line number, or a text search.
- In Org files, you may link to a headline name, a custom ID, or a
- code reference instead.
-
As a special case, "file" prefix may be omitted if the file name
is complete, e.g., it starts with =./=, or =/=.
@@ -3363,44 +3359,50 @@ Here is the full set of built-in link types:
Execute a shell command upon activation.
+
+For =file:= and =id:= links, you can additionally specify a line
+number, or a text search string, separated by =::=. In Org files, you
+may link to a headline name, a custom ID, or a code reference instead.
+
The following table illustrates the link types above, along with their
options:
-| Link Type | Example |
-|------------+----------------------------------------------------------|
-| http | =http://staff.science.uva.nl/c.dominik/= |
-| https | =https://orgmode.org/= |
-| doi | =doi:10.1000/182= |
-| file | =file:/home/dominik/images/jupiter.jpg= |
-| | =/home/dominik/images/jupiter.jpg= (same as above) |
-| | =file:papers/last.pdf= |
-| | =./papers/last.pdf= (same as above) |
-| | =file:/ssh:me@some.where:papers/last.pdf= (remote) |
-| | =/ssh:me@some.where:papers/last.pdf= (same as above) |
-| | =file:sometextfile::NNN= (jump to line number) |
-| | =file:projects.org= |
-| | =file:projects.org::some words= (text search)[fn:12] |
-| | =file:projects.org::*task title= (headline search) |
-| | =file:projects.org::#custom-id= (headline search) |
-| attachment | =attachment:projects.org= |
-| | =attachment:projects.org::some words= (text search) |
-| docview | =docview:papers/last.pdf::NNN= |
-| id | =id:B7423F4D-2E8A-471B-8810-C40F074717E9= |
-| news | =news:comp.emacs= |
-| mailto | =mailto:adent@galaxy.net= |
-| mhe | =mhe:folder= (folder link) |
-| | =mhe:folder#id= (message link) |
-| rmail | =rmail:folder= (folder link) |
-| | =rmail:folder#id= (message link) |
-| gnus | =gnus:group= (group link) |
-| | =gnus:group#id= (article link) |
-| bbdb | =bbdb:R.*Stallman= (record with regexp) |
-| irc | =irc:/irc.com/#emacs/bob= |
-| help | =help:org-store-link= |
-| info | =info:org#External links= |
-| shell | =shell:ls *.org= |
-| elisp | =elisp:(find-file "Elisp.org")= (Elisp form to evaluate) |
-| | =elisp:org-agenda= (interactive Elisp command) |
+| Link Type | Example |
+|------------+--------------------------------------------------------------------|
+| http | =http://staff.science.uva.nl/c.dominik/= |
+| https | =https://orgmode.org/= |
+| doi | =doi:10.1000/182= |
+| file | =file:/home/dominik/images/jupiter.jpg= |
+| | =/home/dominik/images/jupiter.jpg= (same as above) |
+| | =file:papers/last.pdf= |
+| | =./papers/last.pdf= (same as above) |
+| | =file:/ssh:me@some.where:papers/last.pdf= (remote) |
+| | =/ssh:me@some.where:papers/last.pdf= (same as above) |
+| | =file:sometextfile::NNN= (jump to line number) |
+| | =file:projects.org= |
+| | =file:projects.org::some words= (text search)[fn:12] |
+| | =file:projects.org::*task title= (headline search) |
+| | =file:projects.org::#custom-id= (headline search) |
+| attachment | =attachment:projects.org= |
+| | =attachment:projects.org::some words= (text search) |
+| docview | =docview:papers/last.pdf::NNN= |
+| id | =id:B7423F4D-2E8A-471B-8810-C40F074717E9= |
+| | =id:B7423F4D-2E8A-471B-8810-C40F074717E9::*task= (headline search) |
+| news | =news:comp.emacs= |
+| mailto | =mailto:adent@galaxy.net= |
+| mhe | =mhe:folder= (folder link) |
+| | =mhe:folder#id= (message link) |
+| rmail | =rmail:folder= (folder link) |
+| | =rmail:folder#id= (message link) |
+| gnus | =gnus:group= (group link) |
+| | =gnus:group#id= (article link) |
+| bbdb | =bbdb:R.*Stallman= (record with regexp) |
+| irc | =irc:/irc.com/#emacs/bob= |
+| help | =help:org-store-link= |
+| info | =info:org#External links= |
+| shell | =shell:ls *.org= |
+| elisp | =elisp:(find-file "Elisp.org")= (Elisp form to evaluate) |
+| | =elisp:org-agenda= (interactive Elisp command) |
#+cindex: VM links
#+cindex: Wanderlust links
@@ -3461,8 +3463,9 @@ current buffer:
- /Org mode buffers/ ::
For Org files, if there is a =<<target>>= at point, the link points
- to the target. Otherwise it points to the current headline, which
- is also the description.
+ to the target. If there is a named block (using =#+name:=) at
+ point, the link points to that name. Otherwise it points to the
+ current headline, which is also the description.
#+vindex: org-id-link-to-org-use-id
#+cindex: @samp{CUSTOM_ID}, property
@@ -3480,6 +3483,13 @@ current buffer:
timestamp, depending on ~org-id-method~. Later, when inserting the
link, you need to decide which one to use.
+ #+vindex: org-id-link-consider-parent-id
+ When ~org-id-link-consider-parent-id~ is ~t~, parent =ID= properties
+ are considered. This allows linking to specific targets, named
+ blocks, or headlines (which may not have a globally unique =ID=
+ themselves) within the context of a parent headline or file which
+ does.
+
- /Email/News clients: VM, Rmail, Wanderlust, MH-E, Gnus/ ::
#+vindex: org-link-email-description-format
@@ -3753,13 +3763,15 @@ the link completion function like this:
(org-link-set-parameter "type" :complete #'some-completion-function)
#+end_src
-** Search Options in File Links
+** Search Options in File and ID Links
:PROPERTIES:
:DESCRIPTION: Linking to a specific location.
:ALT_TITLE: Search Options
:END:
#+cindex: search option in file links
+#+cindex: search option in id links
#+cindex: file links, searching
+#+cindex: id links, searching
#+cindex: attachment links, searching
File links can contain additional information to make Emacs jump to a
@@ -3771,8 +3783,8 @@ example, when the command ~org-store-link~ creates a link (see
line as a search string that can be used to find this line back later
when following the link with {{{kbd(C-c C-o)}}}.
-Note that all search options apply for Attachment links in the same
-way that they apply for File links.
+Note that all search options apply for Attachment and ID links in the
+same way that they apply for File links.
Here is the syntax of the different ways to attach a search to a file
link, together with explanations for each:
@@ -21177,7 +21189,7 @@ The following =ol-man.el= file implements it
PATH should be a topic that can be thrown at the man command."
(funcall org-man-command path))
-(defun org-man-store-link ()
+(defun org-man-store-link (&optional _interactive?)
"Store a link to a man page."
(when (memq major-mode '(Man-mode woman-mode))
;; This is a man page, we do make this link.
@@ -21237,13 +21249,15 @@ A review of =ol-man.el=:
For example, ~org-man-store-link~ is responsible for storing a link
when ~org-store-link~ (see [[*Handling Links]]) is called from a buffer
- displaying a man page. It first checks if the major mode is
- appropriate. If check fails, the function returns ~nil~, which
- means it isn't responsible for creating a link to the current
- buffer. Otherwise the function makes a link string by combining
- the =man:= prefix with the man topic. It also provides a default
- description. The function ~org-insert-link~ can insert it back
- into an Org buffer later on.
+ displaying a man page. It is passed an argument ~interactive?~
+ which this function does not use, but other store functions use to
+ behave differently when a link is stored interactively by the user.
+ It first checks if the major mode is appropriate. If check fails,
+ the function returns ~nil~, which means it isn't responsible for
+ creating a link to the current buffer. Otherwise the function
+ makes a link string by combining the =man:= prefix with the man
+ topic. It also provides a default description. The function
+ ~org-insert-link~ can insert it back into an Org buffer later on.
** Adding Export Backends
:PROPERTIES:
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 19117821a..6c6171dc7 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -229,6 +229,12 @@ timestamp object. Possible values: ~timerange~, ~daterange~, ~nil~.
~org-element-timestamp-interpreter~ takes into account this property
and returns an appropriate timestamp string.
+**** =org-link= store functions are passed an ~interactive?~ argument
+
+The ~:store:~ functions set for link types using
+~org-link-set-parameters~ are now passed an ~interactive?~ argument,
+indicating whether ~org-store-link~ was called interactively.
+
*** ~org-priority=show~ command no longer adjusts for scheduled/deadline
In agenda views, ~org-priority=show~ command previously displayed the
@@ -307,6 +313,17 @@ The change is breaking when ~org-use-property-inheritance~ is set to ~t~.
*** ~org-babel-lilypond-compile-lilyfile~ ignores optional second argument
The =TEST= parameter is better served by Emacs debugging tools.
+
+*** ~org-id-store-link~ now adds search strings for precise link targets
+
+Previously, search strings are supported for =file:= links but not for
+=id:= links. This change adds support for search strings to =id:=
+links.
+
+This new behaviour can be disabled generally by setting
+~org-id-link-use-context~ to ~nil~, or when storing a specific link by
+passing a prefix argument to ~org-store-link~.
+
** New and changed options
*** New variable ~org-clock-out-removed-last-clock~
@@ -485,6 +502,22 @@ Currently implemented options are:
tasks this technically violates the iCalendar spec, but some
iCalendar programs support this usage.
+*** New option ~org-id-link-consider-parent-id~ to allow =id:= links to parent headlines
+
+For =id:= links, when this option is enabled, ~org-store-link~ will
+look for ids from parent/ancestor headlines, if the current headline
+does not have an id.
+
+Combined with the new ability for =id:= links to use search strings
+for precise link targets (when =org-id-link-use-context= is =t=, which
+is the default), this allows linking to specific headlines without
+requiring every headline to have an id property, as long as the
+headline is unique within a subtree that does have an id property.
+
+By giving files top-level id properties, links to headlines in the
+file can be made more robust by using the file id instead of the file
+path.
+
** New features
*** =ob-plantuml.el=: Support tikz file format output
diff --git a/lisp/ol.el b/lisp/ol.el
index e684b9504..e5938ee13 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -63,7 +63,6 @@
(declare-function org-find-property "org" (property &optional value))
(declare-function org-get-heading "org" (&optional no-tags no-todo no-priority no-comment))
(declare-function org-id-find-id-file "org-id" (id))
-(declare-function org-id-store-link "org-id" ())
(declare-function org-insert-heading "org" (&optional arg invisible-ok top))
(declare-function org-load-modules-maybe "org" (&optional force))
(declare-function org-mark-ring-push "org" (&optional pos buffer))
@@ -819,6 +818,46 @@ spec."
(not (org-in-regexp org-link-any-re))))
\f
+(defun org-link--try-link-store-functions (interactive?)
+ "Try storing external links, prompting if more than one is possible.
+
+Each function returned by `org-store-link-functions` is called in
+turn. If multiple functions return non-nil, prompt for which link
+should be stored.
+
+Return t if a function has successfully stored a link, which will
+be stored in `org-link-store-props`.
+"
+ (let ((results-alist nil))
+ (dolist (f (org-store-link-functions))
+ (when (condition-case nil
+ (funcall f interactive?)
+ ;; XXX: The store function used (< Org 9.7) to accept no
+ ;; arguments; provide backward compatibility support for
+ ;; them.
+ (wrong-number-of-arguments
+ (funcall f)))
+ ;; XXX: return value is not link's plist, so we
+ ;; store the new value before it is modified. It
+ ;; would be cleaner to ask store link functions to
+ ;; return the plist instead.
+ (push (cons f (copy-sequence org-store-link-plist))
+ results-alist)))
+ (pcase results-alist
+ (`nil nil)
+ (`((,_ . ,_)) t) ;single choice: nothing to do
+ (`((,name . ,_) . ,_)
+ ;; Reinstate link plist associated to the chosen
+ ;; function.
+ (apply #'org-link-store-props
+ (cdr (assoc-string
+ (completing-read
+ (format "Store link with (default %s): " name)
+ (mapcar #'car results-alist)
+ nil t nil nil (symbol-name name))
+ results-alist)))
+ t))))
+
;;; Public API
(defun org-link-types ()
@@ -1334,6 +1373,57 @@ priority cookie or tag."
(org-link--normalize-string
(or string (org-get-heading t t t t)))))
+(defun org-link-precise-link-target (&optional relative-to)
+ "Determine search string and description for storing a link.
+
+If a search string is found, return cons cell `(search-string
+. desc)`. Otherwise, return nil.
+
+If there is an active region, the contents is used (see
+`org-link--context-from-region`).
+
+In org-mode buffers, if point is at a named element (e.g. a
+source block), the name is used. If within a heading, the current
+heading is used.
+
+If none of those finds a suitable search string, the current line
+is used as the search string.
+
+Optional argument RELATIVE-TO specifies the buffer position where
+the search will start from. If the search target that would be
+returned is already at this location, return nil to avoid
+unnecessary search strings (for example, when using search
+strings to find targets within org-id links)."
+ (let ((result
+ (cond
+ ((derived-mode-p 'org-mode)
+ (let* ((element (org-element-at-point))
+ (name (org-element-property :name element))
+ (heading (org-element-lineage element 'headline t)))
+ (cond
+ ((let ((region (org-link--context-from-region)))
+ (and region (cons (org-link--normalize-string region t) nil))))
+ (name
+ (cons name name))
+ ((org-before-first-heading-p)
+ (cons (org-link--normalize-string (org-current-line-string) t) nil))
+ ((and heading
+ (> (org-element-begin heading) (or relative-to 0)))
+ (cons (org-link-heading-search-string)
+ (org-link--normalize-string
+ (org-get-heading t t t t)))))))
+
+ ;; Not in an org-mode buffer
+ (t
+ (cons (org-link--normalize-string
+ (or (org-link--context-from-region) (org-current-line-string))
+ t)
+ nil)))))
+
+ ;; Only use search option if there is some text.
+ (when (org-string-nw-p (car result))
+ result)))
+
(defun org-link-open-as-file (path in-emacs)
"Pretend PATH is a file name and open it.
@@ -1560,29 +1650,7 @@ non-nil."
;; available. If more than one can generate a link from current
;; location, ask which one to use.
((and (not (equal arg '(16)))
- (let ((results-alist nil))
- (dolist (f (org-store-link-functions))
- (when (funcall f)
- ;; XXX: return value is not link's plist, so we
- ;; store the new value before it is modified. It
- ;; would be cleaner to ask store link functions to
- ;; return the plist instead.
- (push (cons f (copy-sequence org-store-link-plist))
- results-alist)))
- (pcase results-alist
- (`nil nil)
- (`((,_ . ,_)) t) ;single choice: nothing to do
- (`((,name . ,_) . ,_)
- ;; Reinstate link plist associated to the chosen
- ;; function.
- (apply #'org-link-store-props
- (cdr (assoc-string
- (completing-read
- (format "Store link with (default %s): " name)
- (mapcar #'car results-alist)
- nil t nil nil (symbol-name name))
- results-alist)))
- t))))
+ (org-link--try-link-store-functions interactive?))
(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 nil (ask user).
@@ -1634,6 +1702,7 @@ non-nil."
(org-with-point-at m
(setq agenda-link (org-store-link nil interactive?))))))
+ ;; Calendar mode
((eq major-mode 'calendar-mode)
(let ((cd (calendar-cursor-to-date)))
(setq link
@@ -1642,6 +1711,7 @@ non-nil."
(org-encode-time 0 0 0 (nth 1 cd) (nth 0 cd) (nth 2 cd))))
(org-link-store-props :type "calendar" :date cd)))
+ ;; Image mode
((eq major-mode 'image-mode)
(setq cpltxt (concat "file:"
(abbreviate-file-name buffer-file-name))
@@ -1659,12 +1729,20 @@ non-nil."
(setq cpltxt (concat "file:" file)
link cpltxt)))
+ ;; Try `org-create-file-search-functions`. If any are
+ ;; successful, create a file link to the current buffer with
+ ;; the provided search string. (sets `link` and `cpltxt` to
+ ;; the same thing; it looks like the intention originally was
+ ;; that cpltxt was a description, which might have been set by
+ ;; the search-function (removed in switch to lexical binding)).
((setq search (run-hook-with-args-until-success
'org-create-file-search-functions))
(setq link (concat "file:" (abbreviate-file-name buffer-file-name)
"::" search))
(setq cpltxt (or link))) ;; description
+ ;; Main logic for storing built-in link types in org-mode
+ ;; buffers
((and (buffer-file-name (buffer-base-buffer)) (derived-mode-p 'org-mode))
(org-with-limited-levels
(setq custom-id (org-entry-get nil "CUSTOM_ID"))
@@ -1684,71 +1762,33 @@ non-nil."
desc nil
;; 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?
- (or (eq org-id-link-to-org-use-id 'create-if-interactive)
- (and (eq org-id-link-to-org-use-id
- '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 (plist-get org-store-link-plist :description)))
- (error
- ;; Probably before first headline, link only to file
- (concat "file:"
- (abbreviate-file-name
- (buffer-file-name (buffer-base-buffer))))))))
- (t
+ (t
;; Just link to current headline.
(setq cpltxt (concat "file:"
(abbreviate-file-name
(buffer-file-name (buffer-base-buffer)))))
- ;; Add a context search string.
- (when (org-xor org-link-context-for-files (equal arg '(4)))
- (let* ((element (org-element-at-point))
- (name (org-element-property :name element))
- (context
- (cond
- ((let ((region (org-link--context-from-region)))
- (and region (org-link--normalize-string region t))))
- (name)
- ((org-before-first-heading-p)
- (org-link--normalize-string (org-current-line-string) t))
- (t (org-link-heading-search-string)))))
- (when (org-string-nw-p context)
- (setq cpltxt (format "%s::%s" cpltxt context))
- (setq desc
- (or name
- ;; Although description is not a search
- ;; string, use `org-link--normalize-string'
- ;; to prettify it (contiguous white spaces)
- ;; and remove volatile contents (statistics
- ;; cookies).
- (and (not (org-before-first-heading-p))
- (org-link--normalize-string
- (org-get-heading t t t t)))
- "NONE")))))
- (setq link cpltxt)))))
-
+ (when (org-xor org-link-context-for-files (equal arg '(4)))
+ (pcase (org-link-precise-link-target)
+ (`nil nil)
+ (`(,search-string . ,search-desc)
+ (setq cpltxt (format "%s::%s" cpltxt search-string))
+ (setq desc search-desc))))
+ (setq link cpltxt)))))
+
+ ;; Buffer linked to file, but not an org-mode buffer.
((buffer-file-name (buffer-base-buffer))
;; Just link to this file here.
(setq cpltxt (concat "file:"
(abbreviate-file-name
(buffer-file-name (buffer-base-buffer)))))
;; Add a context search string.
- (when (org-xor org-link-context-for-files (equal arg '(4)))
- (let ((context (org-link--normalize-string
- (or (org-link--context-from-region)
- (org-current-line-string))
- t)))
- ;; Only use search option if there is some text.
- (when (org-string-nw-p context)
- (setq cpltxt (format "%s::%s" cpltxt context))
- (setq desc "NONE"))))
- (setq link cpltxt))
+ (when (org-xor org-link-context-for-files (equal arg '(4)))
+ (pcase (org-link-precise-link-target)
+ (`nil nil)
+ (`(,search-string . ,search-desc)
+ (setq cpltxt (format "%s::%s" cpltxt search-string))
+ (setq desc search-desc))))
+ (setq link cpltxt))
(interactive?
(user-error "No method for storing a link from this buffer"))
diff --git a/lisp/org-id.el b/lisp/org-id.el
index fbe6a0ed0..784d7cb00 100644
--- a/lisp/org-id.el
+++ b/lisp/org-id.el
@@ -129,6 +129,37 @@ nil Never use an ID to make a link, instead link using a text search for
(const :tag "Only use existing" use-existing)
(const :tag "Do not use ID to create link" nil)))
+(defcustom org-id-link-consider-parent-id nil
+ "Non-nil means storing a link to an Org file will consider parent entry IDs.
+
+Use this with `org-id-link-use-context` set to `t` to allow
+linking to uniquely-named sub-entries within a parent entry with
+an ID, without requiring every sub-entry to have its own ID."
+ :group 'org-link-store
+ :group 'org-id
+ :package-version '(Org . "9.7")
+ :type 'boolean)
+
+(defcustom org-id-link-use-context t
+ "Non-nil means org-id links from `org-id-store-link' contain context.
+\\<org-mode-map>
+A search string is added to the id with \"::\" as separator and
+used to find the context when the link is activated by the
+command `org-open-at-point'. When this option is t, the entire
+active region is be placed in the search string of the file link.
+If set to a positive integer N, only the first N lines of context
+are stored.
+
+Using a prefix argument to the commands `org-store-link' \
+\(`\\[universal-argument] \\[org-store-link]') or
+`org-id-store-link` negates this setting for the duration of the
+command."
+ :group 'org-link-store
+ :group 'org-id
+ :package-version '(Org . "9.7")
+ :type '(choice boolean integer)
+ :safe (lambda (val) (or (booleanp val) (integerp val))))
+
(defcustom org-id-uuid-program "uuidgen"
"The uuidgen program."
:group 'org-id
@@ -258,14 +289,17 @@ This variable is only relevant when `org-id-track-globally' is set."
;;; The API functions
;;;###autoload
-(defun org-id-get-create (&optional force)
+(defun org-id-get-create (&optional force inherit)
"Create an ID for the current entry and return it.
If the entry already has an ID, just return it.
-With optional argument FORCE, force the creation of a new ID."
+With optional argument FORCE, force the creation of a new ID.
+With optional argument INHERIT, consider parents' IDs if the
+current entry does not have one."
(interactive "P")
(when force
- (org-entry-put (point) "ID" nil))
- (org-id-get (point) 'create))
+ (org-entry-put (point) "ID" nil)
+ (setq inherit nil))
+ (org-id-get (point) 'create nil inherit))
;;;###autoload
(defun org-id-copy ()
@@ -280,15 +314,16 @@ This is useful when working with contents in a temporary buffer
that will be copied back to the original.")
;;;###autoload
-(defun org-id-get (&optional epom create prefix)
+(defun org-id-get (&optional epom create prefix inherit)
"Get the ID property of the entry at EPOM.
EPOM is an element, marker, or buffer position.
If EPOM is nil, refer to the entry at point.
If the entry does not have an ID, the function returns nil.
+If INHERIT is non-nil, parents' IDs are also considered.
However, when CREATE is non-nil, create an ID if none is present already.
PREFIX will be passed through to `org-id-new'.
In any case, the ID of the entry is returned."
- (let ((id (org-entry-get epom "ID")))
+ (let ((id (org-entry-get epom "ID" inherit)))
(cond
((and id (stringp id) (string-match "\\S-" id))
id)
@@ -704,17 +739,33 @@ optional argument MARKERP, return the position as a new marker."
;; so we do have to add it to `org-store-link-functions'.
;;;###autoload
-(defun org-id-store-link ()
+(defun org-id-store-link (interactive?)
"Store a link to the current entry, using its ID.
+See also `org-id-link-to-org-use-id`,
+`org-id-link-use-context`,
+`org-id-link-consider-parent-id`.
+
If before first heading store first title-keyword as description
or filename if no title."
- (interactive)
- (when (and (buffer-file-name (buffer-base-buffer)) (derived-mode-p 'org-mode))
- (let* ((link (concat "id:" (org-id-get-create)))
+ (interactive "p")
+ (when (and (buffer-file-name (buffer-base-buffer))
+ (derived-mode-p 'org-mode)
+ (or (eq org-id-link-to-org-use-id t)
+ (and interactive?
+ (or (eq org-id-link-to-org-use-id 'create-if-interactive)
+ (and (eq org-id-link-to-org-use-id
+ 'create-if-interactive-and-no-custom-id)
+ (not (org-entry-get nil "CUSTOM_ID")))))
+ (and org-id-link-to-org-use-id
+ (org-entry-get nil "ID" org-id-link-consider-parent-id))))
+ (let* ((link (concat "id:" (org-id-get-create nil org-id-link-consider-parent-id)))
+ (id-location (or (and org-entry-property-inherited-from
+ (marker-position org-entry-property-inherited-from))
+ (save-excursion (org-back-to-heading-or-point-min) (point))))
(case-fold-search nil)
(desc (save-excursion
- (org-back-to-heading-or-point-min t)
+ (goto-char id-location)
(cond ((org-before-first-heading-p)
(let ((keywords (org-collect-keywords '("TITLE"))))
(if keywords
@@ -726,14 +777,25 @@ or filename if no title."
(match-string 4)
(match-string 0)))
(t link)))))
+ ;; Prefix to `org-store-link` negates preference from `org-id-link-use-context`.
+ (when (org-xor current-prefix-arg org-id-link-use-context)
+ (pcase (org-link-precise-link-target id-location)
+ (`nil nil)
+ (`(,search-string . ,search-desc)
+ (setq link (concat link "::" search-string))
+ (setq desc search-desc))))
(org-link-store-props :link link :description desc :type "id")
link)))
(defun org-id-open (id _)
"Go to the entry with id ID."
- (org-mark-ring-push)
- (let ((m (org-id-find id 'marker))
- cmd)
+ (let* ((option (and (string-match "::\\(.*\\)\\'" id)
+ (match-string 1 id)))
+ (id (if (not option) id
+ (substring id 0 (match-beginning 0))))
+ m cmd)
+ (org-mark-ring-push)
+ (setq m (org-id-find id 'marker))
(unless m
(error "Cannot find entry with ID \"%s\"" id))
;; Use a buffer-switching command in analogy to finding files
@@ -750,9 +812,13 @@ or filename if no title."
(funcall cmd (marker-buffer m)))
(goto-char m)
(move-marker m nil)
+ (when option
+ (org-link-search option))
(org-fold-show-context)))
-(org-link-set-parameters "id" :follow #'org-id-open)
+(org-link-set-parameters "id"
+ :follow #'org-id-open
+ :store #'org-id-store-link)
(provide 'org-id)
diff --git a/testing/lisp/test-ol.el b/testing/lisp/test-ol.el
index e0cec0854..24d926084 100644
--- a/testing/lisp/test-ol.el
+++ b/testing/lisp/test-ol.el
@@ -381,6 +381,140 @@ See https://github.com/yantar92/org/issues/4."
(equal (format "[[file:%s::*foo bar][foo bar]]" file file)
(org-store-link nil)))))))
+(ert-deftest test-org-link/precise-link-target ()
+ "Test `org-link-precise-link-target` specifications."
+ (should
+ (equal '("*H1" . "H1")
+ (org-test-with-temp-text "* H1<point>\n* H2\n"
+ (org-link-precise-link-target))))
+ (should
+ (equal '("foo" . "foo")
+ (org-test-with-temp-text "* H1\n#+name: foo<point>\n#+begin_example\nhi\n#+end_example\n"
+ (org-link-precise-link-target))))
+ (should
+ (equal '("Text" . nil)
+ (org-test-with-temp-text "\nText<point>\n* H1\n"
+ (org-link-precise-link-target))))
+ (should
+ (equal nil
+ (org-test-with-temp-text "\n<point>\n* H1\n"
+ (org-link-precise-link-target))))
+ ;; relative to a heading
+ (should
+ (equal nil
+ (org-test-with-temp-text "* H1<point>\n* H2\n"
+ (org-link-precise-link-target 1))))
+ (should
+ (equal '("*H2" . "H2")
+ (org-test-with-temp-text "* H1\n* H2<point>\n"
+ (org-link-precise-link-target 1))))
+ (should
+ (equal nil
+ (org-test-with-temp-text "* H1\n* H2<point>\n"
+ (org-link-precise-link-target 6))))
+ )
+
+(defmacro test-ol-stored-link-with-text (text &rest body)
+ "Return :link and :description from link stored in body."
+ (declare (indent 1))
+ `(let (org-store-link-plist)
+ (org-test-with-temp-text-in-file ,text
+ ,@body
+ (list (plist-get org-store-link-plist :link)
+ (plist-get org-store-link-plist :description)))))
+
+(ert-deftest test-org-link/id-store-link ()
+ "Test `org-id-store-link' specifications."
+ (let ((org-id-link-to-org-use-id nil))
+ (should
+ (equal '(nil nil)
+ (test-ol-stored-link-with-text "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n"
+ (org-id-store-link t)))))
+ ;; On a headline, link to that headline's ID. Use heading as the
+ ;; description of the link.
+ (let ((org-id-link-to-org-use-id t))
+ (should
+ (equal '("id:abc" "H1")
+ (test-ol-stored-link-with-text "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n"
+ (org-id-store-link t)))))
+ ;; Remove TODO keywords etc from description of the link.
+ (let ((org-id-link-to-org-use-id t))
+ (should
+ (equal '("id:abc" "H1")
+ (test-ol-stored-link-with-text "* TODO [#A] H1 :tag:\n:PROPERTIES:\n:ID: abc\n:END:\n"
+ (org-id-store-link t)))))
+ ;; create-if-interactive
+ (let ((org-id-link-to-org-use-id 'create-if-interactive))
+ (should
+ (equal '("id:abc" "H1")
+ (cl-letf (((symbol-function 'org-id-new)
+ (lambda (&rest _rest) "abc")))
+ (test-ol-stored-link-with-text "* H1\n"
+ (org-id-store-link t)))))
+ (should
+ (equal '(nil nil)
+ (test-ol-stored-link-with-text "* H1\n"
+ (org-id-store-link nil)))))
+ ;; create-if-interactive-and-no-custom-id
+ (let ((org-id-link-to-org-use-id 'create-if-interactive-and-no-custom-id))
+ (should
+ (equal '("id:abc" "H1")
+ (cl-letf (((symbol-function 'org-id-new)
+ (lambda (&rest _rest) "abc")))
+ (test-ol-stored-link-with-text "* H1\n"
+ (org-id-store-link t)))))
+ (should
+ (equal '(nil nil)
+ (test-ol-stored-link-with-text "* H1\n:PROPERTIES:\n:CUSTOM_ID: xyz\n:END:\n"
+ (org-id-store-link t))))
+ (should
+ (equal '(nil nil)
+ (test-ol-stored-link-with-text "* H1\n"
+ (org-id-store-link nil))))))
+
+(ert-deftest test-org-link/id-store-link-using-parent ()
+ "Test `org-id-store-link' specifications with `org-id-link-consider-parent-id` set."
+ (let ((org-id-link-to-org-use-id 'use-existing)
+ (org-id-link-consider-parent-id nil))
+ (should
+ (equal '(nil nil)
+ (test-ol-stored-link-with-text "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n** H2\n<point>"
+ (org-id-store-link t)))))
+ ;; when using context to still find specific heading
+ (let ((org-id-link-to-org-use-id 'use-existing)
+ (org-id-link-consider-parent-id t)
+ (org-id-link-use-context t))
+ (should
+ (equal '("id:abc::*H2" "H2")
+ (test-ol-stored-link-with-text "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n** H2\n<point>"
+ (org-id-store-link t))))
+ (should
+ (equal '("id:abc::name" "name")
+ (test-ol-stored-link-with-text "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n\n#+name: name\n<point>#+begin_example\nhi\n#+end_example\n"
+ (org-id-store-link t))))
+ (should
+ (equal '("id:abc" "H1")
+ (test-ol-stored-link-with-text "* H1<point>\n:PROPERTIES:\n:ID: abc\n:END:\n** H2\n"
+ (org-id-store-link t)))))
+ ;; when not using context, description should be the parent/file
+ (let ((org-id-link-to-org-use-id 'use-existing)
+ (org-id-link-consider-parent-id t)
+ (org-id-link-use-context nil))
+ (should
+ (equal '("id:abc" "H1")
+ (test-ol-stored-link-with-text "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n** H2\n<point>"
+ (org-id-store-link t))))
+ (should
+ (let ((result (test-ol-stored-link-with-text ":PROPERTIES:\n:ID: top\n:END:\n:* H1\n<point>"
+ (org-id-store-link t))))
+ (equal "id:top" (car result))
+ ;; strip random buffer file name
+ (equal "org-test" (substring (cadr result) 0 8))))
+ (should
+ (equal '("id:top" "title")
+ (test-ol-stored-link-with-text ":PROPERTIES:\n:ID: top\n:END:\n#+TITLE: title\n\n:* H1\n<point>"
+ (org-id-store-link t))))))
+
\f
;;; Radio Targets
--
2.39.2 (Apple Git-143)
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-11-19 15:21 ` Rick Lupton
@ 2023-12-04 13:23 ` Rick Lupton
2023-12-10 13:35 ` Ihor Radchenko
1 sibling, 0 replies; 16+ messages in thread
From: Rick Lupton @ 2023-12-04 13:23 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: Y. E.
Hi,
I can’t see this patch listed at https://tracker.orgmode.org/ so just wanted to check it hasn’t got lost?
Thanks
Rick
On Sun, 19 Nov 2023, at 3:21 PM, Rick Lupton wrote:
> Here's an updated patch, which adds (optional) search strings to ID
> links, and the option to inherit ID targets from parent headline / the
> top level file properties. I've also updated ORG-NEWS and the manual,
> and added tests.
>
> I think I've fixed all the issues with my first patch about which
> headline gets used for the description when inheriting IDs, what
> happens if there is no ID, etc.
>
>> Ideally, we should have all the necessary logic to store the link within `org-id-store-link' and then use `org-link-set-parameters' to configure id links.
>> ...
>> I think that we need to make a change in the rules for :store functions. `interactive?' may be passed as the argument to these functions.
>
> I've also moved the org-id specific logic from `org-store-link` to
> `org-id-store-link`, and added the `interactive?` argument to link
> store functions as discussed.
>
>>> So my question is: should search strings be added to all org-id links?
>> Sounds as a reasonable default, but users should have an option to revert to previous behaviour with heading id being stored.
>
> The default value for the new option `org-id-link-use-context` is `t`,
> but it can be set to `nil` (or disabled with a prefix argument to
> `org-store-link` temporarily). This is a change in default behaviour
> when storing ID links with point at a subheading, named block, or
> target, or with an active region.
>
> The option `org-id-link-consider-parent-id` I've left with a default
> value of `nil`, since I'm not sure if everyone will want this behaviour.
>
> Thanks
> Rick
>
> Attachments:
> * 0001-org-id.el-Extend-links-with-search-strings-inherit-p.patch
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] org-id: allow using parent's existing id in links to headlines
2023-11-19 15:21 ` Rick Lupton
2023-12-04 13:23 ` Rick Lupton
@ 2023-12-10 13:35 ` Ihor Radchenko
1 sibling, 0 replies; 16+ messages in thread
From: Ihor Radchenko @ 2023-12-10 13:35 UTC (permalink / raw)
To: Rick Lupton; +Cc: emacs-orgmode list
"Rick Lupton" <mail@ricklupton.name> writes:
> Here's an updated patch, which adds (optional) search strings to ID links, and the option to inherit ID targets from parent headline / the top level file properties. I've also updated ORG-NEWS and the manual, and added tests.
>
> I think I've fixed all the issues with my first patch about which headline gets used for the description when inheriting IDs, what happens if there is no ID, etc.
Thanks!
> +*** ~org-id-store-link~ now adds search strings for precise link targets
> +
> +Previously, search strings are supported for =file:= links but not for
> +=id:= links. This change adds support for search strings to =id:=
> +links.
"This change..." part sounds like a commit message, not a NEWS item.
You may probably remove this paragraph.
> \f
> +(defun org-link--try-link-store-functions (interactive?)
> + "Try storing external links, prompting if more than one is possible.
> +
> +Each function returned by `org-store-link-functions` is called in
> +turn. If multiple functions return non-nil, prompt for which link
> +should be stored.
We use `...' Elisp quoting, not markdown.
Also, please keep two spaces between sentences - you do it
inconsistently across the patch.
> +Return t if a function has successfully stored a link, which will
> +be stored in `org-link-store-props`.
I'd better say "Return t when the link is stored in `org-store-link-plist'."
> +(defcustom org-id-link-consider-parent-id nil
> + "Non-nil means storing a link to an Org file will consider parent entry IDs.
> +
> +Use this with `org-id-link-use-context` set to `t` to allow
> +linking to uniquely-named sub-entries within a parent entry with
> +an ID, without requiring every sub-entry to have its own ID."
1. `...' quoting
2. The docstring is slightly confusing. Having an example would be helpful.
> +(defcustom org-id-link-use-context t
> + "Non-nil means org-id links from `org-id-store-link' contain context.
> +\\<org-mode-map>
> +A search string is added to the id with \"::\" as separator and
> +used to find the context when the link is activated by the
> +command `org-open-at-point'. When this option is t, the entire
> +active region is be placed in the search string of the file link.
> +If set to a positive integer N, only the first N lines of context
> +are stored.
It does not look like integer value is respected in the patch.
> +Using a prefix argument to the commands `org-store-link' \
> +\(`\\[universal-argument] \\[org-store-link]') or
> +`org-id-store-link` negates this setting for the duration of the
> +command."
You should also update the docstring of `org-store-link' accordingly.
> ;;;###autoload
> -(defun org-id-get-create (&optional force)
> +(defun org-id-get-create (&optional force inherit)
>...
> ;;;###autoload
> -(defun org-id-get (&optional epom create prefix)
> +(defun org-id-get (&optional epom create prefix inherit)
Please document this new optional arguments in the NEWS.
> - (let ((id (org-entry-get epom "ID")))
> + (let ((id (org-entry-get epom "ID" inherit)))
This makes your description of INHERIT argument slightly inaccurate - for
`org-entry-get', INHERIT can also be a special symbol 'selective.
> -(defun org-id-store-link ()
> +(defun org-id-store-link (interactive?)
Please make this new argument optional and document the argument in the
docstring and NEWS. Non-optional new argument is a breaking change that
may break third-party code.
> "Store a link to the current entry, using its ID.
>
> +See also `org-id-link-to-org-use-id`,
> +`org-id-link-use-context`,
> +`org-id-link-consider-parent-id`.
> +
> If before first heading store first title-keyword as description
> or filename if no title."
> - (interactive)
> - (when (and (buffer-file-name (buffer-base-buffer)) (derived-mode-p 'org-mode))
> - (let* ((link (concat "id:" (org-id-get-create)))
> + (interactive "p")
> + (when (and (buffer-file-name (buffer-base-buffer))
> + (derived-mode-p 'org-mode)
> + (or (eq org-id-link-to-org-use-id t)
I do not like this change - `org-id-store-link' is not only used by
`org-store-link'. Suddenly honoring `org-id-link-to-org-use-id' will be
a breaking change. Instead, I suggest you to write a wrapper function,
like `org-id-store-link-maybe' and use it as :store id link property.
That function will call `org-id-store-link' as necessary according to
user customization.
> + ;; Prefix to `org-store-link` negates preference from `org-id-link-use-context`.
> + (when (org-xor current-prefix-arg org-id-link-use-context)
This is not reliable. `org-id-store-link' may be called from a completely
different command, not `org-store-link'. Then, the effect of prefix
argument will be unexpected. You should instead process prefix argument
right in `org-store-link' by let-binding `org-id-link-use-context'
around the call to `org-id-store-link'.
> + (pcase (org-link-precise-link-target id-location)
Why not passing the RELATIVE-TO argument?
> (defun org-id-open (id _)
> "Go to the entry with id ID."
> - (org-mark-ring-push)
> - (let ((m (org-id-find id 'marker))
> - cmd)
> + (let* ((option (and (string-match "::\\(.*\\)\\'" id)
> + (match-string 1 id)))
> + (id (if (not option) id
> + (substring id 0 (match-beginning 0))))
> + m cmd)
> + (org-mark-ring-push)
> + (setq m (org-id-find id 'marker))
This means that the existing IDs that happen to contain :: will be
broken. For such IDs, we should (1) document the problem in the news;
(2) try harder to match them calling `org-id-find' with all the possible
ID values until one matches.
> + (when option
> + (org-link-search option))
> (org-fold-show-context)))
`org-link-search' does not always search from point. So, you may end up
matching, for example, a duplicate CUSTOM_ID above.
Moreover, regular expression match option will be broken -
`org-link-search' creates sparse tree in the whole buffer and will
disregard the ID part of the link. I suspect that you will need to make
dedicated modifications to `org-link-search' as well in order to
implement opening ID links with search option cleanly.
--
Ihor Radchenko // yantar92,
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-12-10 13:33 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-24 11:40 [PATCH] org-id: allow using parent's existing id in links to headlines Rick Lupton
2023-07-25 7:43 ` Ihor Radchenko
2023-07-25 15:16 ` Max Nikulin
2023-07-26 8:10 ` Ihor Radchenko
2023-07-27 0:16 ` Samuel Wales
2023-07-27 7:42 ` IDs below headline level (for paragraphs, lists, etc) (was: [PATCH] org-id: allow using parent's existing id in links to headlines) Ihor Radchenko
2023-07-28 20:00 ` Rick Lupton
2023-07-28 19:56 ` [PATCH] org-id: allow using parent's existing id in links to headlines Rick Lupton
2023-07-29 8:33 ` Ihor Radchenko
2023-11-09 20:56 ` Rick Lupton
2023-11-10 10:03 ` Ihor Radchenko
2023-11-19 15:21 ` Rick Lupton
2023-12-04 13:23 ` Rick Lupton
2023-12-10 13:35 ` Ihor Radchenko
2023-11-04 23:01 ` Rick Lupton
2023-11-05 12:31 ` Ihor Radchenko
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).