Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines

[Ihor Radchenko <yantar92@posteo.net>]

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

"Rick Lupton" <mail@ricklupton.name> writes:

>> It looks like we cannot simply rely on narrowing to determine the
>> created heading level.
>
> I think you're right.  I have extended `org-link-search' to accept an optional argument describing the org element where newly created headings should go as subheadings.
>
> My thought was that this was not significantly more complicated than just passing the numeric level for new headings, but actually more flexible (e.g. you could if you wanted (with additional future elisp) create missing headings as part of a "To be filed" subtree within the file, rather than always at the end).
>
> Does that look ok?

Yes.

> [is it useful to keep attaching the unchanged first patch so they are available as a set?]

Yes, it is useful. Makes it easier for my to batch-apply the patchset
using https://git.kyleam.com/piem/about/

I have some thoughts about rewording your changes to the manual and
ORG-NEWS. See the attached patch on top of yours.

> Subject: [PATCH 1/2] lisp/org.el (org-insert-heading): allow specifying
>  heading level

*Allow

> * lisp/org.el (org-insert-heading): Change optional argument TOP to
> LEVEL, accepting a number to force a specific heading level.
> * testing/lisp/test-org.el (test-org/insert-heading): Add tests
> * etc/ORG-NEWS: Document changes

Please end sentences with period.

> From d5759dd95bec88be38ddbde07fa4437c0528469a Mon Sep 17 00:00:00 2001
> From: Rick Lupton <mail@ricklupton.name>
> Date: Sun, 19 Nov 2023 14:52:05 +0000
> Subject: [PATCH 2/2] org-id.el: Add search strings, inherit parent IDs
>
> ...
> (org-link-try-link-store-functions): Extract logic to call external
> link store functions. Pass them a new `interactive?' argument.
> ...
> (org-id-store-link): 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.

Please, use two spaces between sentences.

> * lisp/org-lint.el: add checker for "::" in ID properties.

... and start sentences from capital letter: *Add

> -(defun org-link-search (s &optional avoid-pos stealth)
> +(defun org-link-search (s &optional avoid-pos stealth new-heading-container)

The new optional argument to a public function should be announced in ORG-NEWS.

> +             (new-heading-level (if new-heading-container
> +                                    (+ 1 (org-element-property :level new-heading-container))

What if new-heading-container is not a heading?

> +                                  1)))
> +        (goto-char new-heading-position)

This is err when container ends after narrowed region boundary.

> +(defun org-link-precise-link-target ()
> ...
> +              (cond
> +               (name
> +                (list name
> +                      name
> +                      (org-element-begin element)))

It would make sense to use #+caption as default description when available.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Amendments-to-org-manual.org-and-ORG-NEWS.patch --]
[-- Type: text/x-patch, Size: 4327 bytes --]

From 0f9a4503d95f7682229ae1c1ad8a4e2d069fc644 Mon Sep 17 00:00:00 2001
Message-ID: <0f9a4503d95f7682229ae1c1ad8a4e2d069fc644.1707396844.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Thu, 8 Feb 2024 13:53:44 +0100
Subject: [PATCH] Amendments to org-manual.org and ORG-NEWS

---
 doc/org-manual.org | 18 ++++++++++--------
 etc/ORG-NEWS       | 27 ++++++++++++++-------------
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 49fce9113..e933a2d63 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -3489,14 +3489,16 @@ ** Handling Links
 
   #+vindex: org-id-link-consider-parent-id
   #+vindex: org-id-link-use-context
-  When ~org-id-link-consider-parent-id~ is ~t~ (and
-  ~org-link-context-for-files~ and ~org-id-link-use-context~ are both
-  enabled), 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.
-
-  For example, given this org file with those variables set:
+  #+vindex: org-link-context-for-files
+  When ~org-id-link-consider-parent-id~ is ~t~[fn:: Also,
+  ~org-link-context-for-files~ and ~org-id-link-use-context~ should be
+  both enabled (which they are, by default).], 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.
+
+  For example, given this org file:
 
   #+begin_src org
   ,* Parent
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 84bbc5243..e29d2895f 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -477,22 +477,23 @@ The change is breaking when ~org-use-property-inheritance~ is set to ~t~.
 
 The =TEST= parameter is better served by Emacs debugging tools.
 
-*** ~org-id-store-link~ now adds search strings for precise link targets
+*** =id:= links support search options; ~org-id-store-link~ adds search option by default
 
-This new behaviour can be disabled generally by setting
-~org-id-link-use-context~ to ~nil~, or the setting can be toggled for
-a single call to ~org-store-link~ with a universal argument.
+Adding search option by ~org-id-store-link~ can be disabled by setting
+~org-id-link-use-context~ to ~nil~, or toggled for a single call by
+passing universal argument.
 
 When using this feature, IDs should not include =::=, which is used in
 links to indicate the start of the search string.  For backwards
 compability, existing IDs including =::= will still be matched (but
-cannot be used together with precise link targets).  An org-lint
-checker has been added to warn about this.
+cannot be used together with search option).  A new org-lint checker
+has been added to warn about this.
 
 *** ~org-store-link~ behaviour storing additional =CUSTOM_ID= links has changed
 
-As well as an =id:= link, ~org-store-link~ stores an additional "human
-readable" link using a node's =CUSTOM_ID= property, if available.
+Previously, when storing =id:= link, ~org-store-link~ stored an
+additional "human readable" link using a node's =CUSTOM_ID= property.
+
 This behaviour has been expanded to store an additional =CUSTOM_ID=
 link when storing any type of external link type in an Org file, not
 just =id:= links.
@@ -778,11 +779,11 @@ 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.
+Combined with the new ability for =id:= links to use search options
+ [fn:: 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.
 
 For example, given this org file:
 
-- 
2.43.0


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


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