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

[Ihor Radchenko]

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

>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

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