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

2024-03-05 Thread Ihor Radchenko
Stefan  writes:

> since this patch was applied, there are now two functions in 
> `org-store-link-functions` that feel responsible for id links from some 
> buffers: the new `org-id-store-link-maybe` and `org-contacts-link-store`.
>
> This results in a prompt asking which one to use, which happens many times, 
> e.g., when exporting agendas with org-contacts stuff in them.
>
> Not sure how to improve/avoid that, though. 

Org mode does not add the two functions there. It is likely something
inside your config.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2024-03-05 Thread Stefan
Hi,

since this patch was applied, there are now two functions in 
`org-store-link-functions` that feel responsible for id links from some 
buffers: the new `org-id-store-link-maybe` and `org-contacts-link-store`.

This results in a prompt asking which one to use, which happens many times, 
e.g., when exporting agendas with org-contacts stuff in them.

Not sure how to improve/avoid that, though. 

Best,
 Stefan



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

2024-02-24 Thread Rick Lupton
Thanks for your help with it!

On Sat, 24 Feb 2024, at 1:02 PM, Ihor Radchenko wrote:
> Bastien Guerry  writes:
>
>>> "Rick Lupton"  writes:
>>>
 On Fri, 9 Feb 2024, at 12:09 PM, Ihor Radchenko wrote:
> May you please update on your FSF copyright assignment status?

 I believe the agreement is all signed and completed. 
>>>
>>> Bastien, may your please check FSF records?
>>
>> Done, and it is well in order.
>
> Thanks for checking!
> Applied, onto main.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6e7e0b2cd
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=95554543b
>
> and added a record to the contributor list.
> https://git.sr.ht/~bzg/worg/commit/0ccaf58a
>
> Rick, thanks for your contribution!
>
> -- 
> Ihor Radchenko // yantar92,
> Org mode contributor,
> Learn more about Org mode at .
> Support Org development at ,
> or support my work at 



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

2024-02-24 Thread Ihor Radchenko
Bastien Guerry  writes:

>> "Rick Lupton"  writes:
>>
>>> On Fri, 9 Feb 2024, at 12:09 PM, Ihor Radchenko wrote:
 May you please update on your FSF copyright assignment status?
>>>
>>> I believe the agreement is all signed and completed. 
>>
>> Bastien, may your please check FSF records?
>
> Done, and it is well in order.

Thanks for checking!
Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6e7e0b2cd
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=95554543b

and added a record to the contributor list.
https://git.sr.ht/~bzg/worg/commit/0ccaf58a

Rick, thanks for your contribution!

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2024-02-24 Thread Bastien Guerry
Hi,

Ihor Radchenko  writes:

> "Rick Lupton"  writes:
>
>> On Fri, 9 Feb 2024, at 12:09 PM, Ihor Radchenko wrote:
>>> May you please update on your FSF copyright assignment status?
>>
>> I believe the agreement is all signed and completed. 
>
> Bastien, may your please check FSF records?

Done, and it is well in order.

-- 
 Bastien Guerry



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

2024-02-09 Thread Ihor Radchenko
"Rick Lupton"  writes:

> On Fri, 9 Feb 2024, at 12:09 PM, Ihor Radchenko wrote:
>> May you please update on your FSF copyright assignment status?
>
> I believe the agreement is all signed and completed. 

Bastien, may your please check FSF records?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2024-02-09 Thread Rick Lupton
On Fri, 9 Feb 2024, at 12:09 PM, Ihor Radchenko wrote:
> May you please update on your FSF copyright assignment status?

I believe the agreement is all signed and completed. 

Thanks
Rick



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

2024-02-09 Thread Ihor Radchenko
"Rick Lupton"  writes:

> On Thu, 8 Feb 2024, at 1:02 PM, Ihor Radchenko wrote:
>> I have some thoughts about rewording your changes to the manual and
>> ORG-NEWS. See the attached patch on top of yours.
>
> Thanks, makes sense -- wasn't sure whether to keep this as a separate patch 
> or not, I have squashed into the attached updated version.

Right. That was intended for squash. I sent it as a separate patch to
make it easier what exactly I proposed to change.

>> It would make sense to use #+caption as default description when available.
>
> Maybe... But I had a little look and it seems complicated, since caption is a 
> parsed property, it's not clear to me how to get a plain string in a simple 
> way. And there could be a long and a short caption, over multiple lines. If 
> the caption is long, it wouldn't make a good link description anyway.
>
> The current behaviour is the same as it was before, so maybe we can leave 
> this as a future enhancement if wanted?

No problem.

I have no further comments on this version of the patch. It is ready for
merging.

May you please update on your FSF copyright assignment status?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2024-02-08 Thread Rick Lupton
On Thu, 8 Feb 2024, at 1:02 PM, Ihor Radchenko wrote:
> I have some thoughts about rewording your changes to the manual and
> ORG-NEWS. See the attached patch on top of yours.

Thanks, makes sense -- wasn't sure whether to keep this as a separate patch or 
not, I have squashed into the attached updated version.

> [minor points on commit messages]

Fixed these.

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

Added.

>> + (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.

Added checks for these.

>> +(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.

Maybe... But I had a little look and it seems complicated, since caption is a 
parsed property, it's not clear to me how to get a plain string in a simple 
way. And there could be a long and a short caption, over multiple lines. If the 
caption is long, it wouldn't make a good link description anyway.

The current behaviour is the same as it was before, so maybe we can leave this 
as a future enhancement if wanted?


0001-lisp-org.el-org-insert-heading-Allow-specifying-head.patch
Description: Binary data


0002-org-id.el-Add-search-strings-inherit-parent-IDs.patch
Description: Binary data


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

2024-02-08 Thread Ihor Radchenko
"Rick Lupton"  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 
> Date: Sun, 19 Nov 2023 14:52:05 +
> 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  avoid-pos stealth)
> +(defun org-link-search (s  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.yanta...@posteo.net>
From: Ihor Radchenko 
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 

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

2024-02-08 Thread Rick Lupton
On Sat, 3 Feb 2024, at 1:10 PM, Ihor Radchenko wrote:
> I'd prefer to avoid using global variables here.
> `org-entry-property-inherited-from' dates to pre-lexical binding times
> and is a potential source of subtle bugs if several `org-entry-get'
> calls happen unexpectedly to the code, changing
> `org-entry-property-inherited-from' multiple times.
>
> Instead, I suggest changing the return value of
> `org-link-precise-link-target' to a list that includes marker in
> addition to search string and description.

Makes sense -- I changed it to work that way and it is neater.

I returned simply the buffer position rather than a marker, since it is always 
in the current buffer, and avoids needing to worry about cleaning up the marker 
when finished or if not of interest.

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

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

Thanks
Rick

0001-lisp-org.el-org-insert-heading-allow-specifying-head.patch
Description: Binary data


0002-org-id.el-Add-search-strings-inherit-parent-IDs.patch
Description: Binary data


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

2024-02-03 Thread Ihor Radchenko
Samuel Wales  writes:

>> May you please elaborate what you want to add to the manual and where?
>
> had been merely thinking mentioning non-brittleness for newcomers.
> in handling links.

Isn't it already mentioned?

 ...  In addition or alternatively, depending on the value of
 ‘org-id-link-to-org-use-id’, create and/or use a globally unique
 ‘ID’ property for the link(1).  So using this command in Org
 buffers potentially creates two links: a human-readable link from
 the custom ID, and one that is globally unique and works even if
 the entry is moved from file to file.  The ‘ID’ property can be
 either a UUID (default) or a timestamp, depending on
 ‘org-id-method’.  Later, when inserting the link, you need to
 decide which one to use.

> but MAYBE also org-id could be slightly more integrated in org, such
> as not having to load the library.

The default value of `org-modules' is indeed a subject of discussion.
AFAIK, the current problem is that adding extra link types into
`org-modules' significantly slows down loading Org.

> ...  (info "(org) Include Files") does not
> mention whether org-id works.  could eliminate the need to specify
> file.

May you elaborate?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2024-02-03 Thread Ihor Radchenko
"Rick Lupton"  writes:

> I see.  Updated to get the search string first, before the possible 
> properties draw appears.
>
> To make this work I changed `org-link-precise-link-target': instead of
> accepting the RELATIVE-TO argument and rejecting unsuitable targets
> internally, it now sets a marker `org-link-precise-target-marker'
> showing where the target that was found is, so the caller can decide
> if the found target is suitable. I copied the approach from
> `org-entry-property-inherited-from', hope that doesn't cause any other
> issues.

I'd prefer to avoid using global variables here.
`org-entry-property-inherited-from' dates to pre-lexical binding times
and is a potential source of subtle bugs if several `org-entry-get'
calls happen unexpectedly to the code, changing
`org-entry-property-inherited-from' multiple times.

Instead, I suggest changing the return value of
`org-link-precise-link-target' to a list that includes marker in
addition to search string and description.

>> The fact that links stored via `org-store-link' cannot be open with
>> default settings is not good.
>> Also, your patch disregards this setting - it should not match
>> non-headline search strings with the default value of
>> `org-link-search-must-match-exact-headline'.
>
> `org-link-search-must-match-exact-headline' affects `org-link-search', which 
> is called by `org-id-open' -- so I think the behaviour for these org-id links 
> should be the same as for other file links? Am I missing something?

No, you don't.
In my testing, I used #+name: as link target.
However, what I missed is that #+name targets are matched even when
`org-link-search-must-match-exact-headline' is set to 'query-to-create.
The docstring is not accurate there and must be updated.

> Or, maybe you mean links that rely on 
> `org-link-search-must-match-exact-headline' should not be stored.  That would 
> seem reasonable, but also doesn't need to be part of these changes here?

Yes, I also meant this. Indeed, it is out of scope of your patch. It was
a comment for future reference.

>> Probably, changing the default value of
>> `org-link-search-must-match-exact-headline' to nil is due.
>
> It seems like the behaviour below would be desirable, but doesn't currently 
> exist with any setting of `org-link-search-must-match-exact-headline'?
>
> (org-link-search "plain text")  -->  fuzzy search for all text
> (org-link-search "*heading")-->  search only headings, optionally 
> creating if missing

That would also make sense. I like this idea.

>>> -  (org-insert-heading nil t t)
>>> +  ;; Find appropriate level for new heading
>>> +  (let ((level (save-excursion
>>> + (goto-char (point-min))
>>> + (+ 1 (or (org-current-level) 0)
>>
>> This is fragile. You assume that `point-min' always contains a heading.
>> That may or may not be the case - `org-link-search' may be called by
>> third-party code that does not care about setting narrowing in certain
>> ways.
>
> I don't think it's a problem. (org-current-level) returns something suitable 
> whether or not point-min contains a heading. Both the situations below seem 
> reasonable choices for the level of the newly created heading at the end:

That's right.

> ---start of narrowing---
> Text
> * H1
> ** H2
> * A new level 1 heading is created at the end
> ---end of narrowing---
>
> ---start of narrowing---
> * H1
> ** H2
> ** A new level 2 heading is created at the end
> ---end of narrowing---

However, the second scenario is unexpected - consider that your
narrowing is not a narrowing but the whole contents of an Org file.

Before your patch, in both cases, a new level 1 heading is created.
With your patch, the second case will create a new level 2 heading even
for [[*Foo]] links.

It looks like we cannot simply rely on narrowing to determine the
created heading level.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2024-02-01 Thread Rick Lupton
On Thu, 1 Feb 2024, at 12:13 PM, Ihor Radchenko wrote:
> The patch does not apply onto the latest main. May you please update it?

I have rebased onto the latest main. It changes quickly!

(there were no conflicts during the rebase, which I'd have thought would mean 
the patches shouldn't be a problem to apply? It the problem was something else, 
please let me know)

0001-lisp-org.el-org-insert-heading-allow-specifying-head.patch
Description: Binary data


0002-org-id.el-Extend-links-with-search-strings-inherit-p.patch
Description: Binary data


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

2024-02-01 Thread Ihor Radchenko


The patch does not apply onto the latest main. May you please update it?



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

2024-01-31 Thread Rick Lupton
On Mon, 29 Jan 2024, at 1:00 PM, Ihor Radchenko wrote:
>>> 3. Consider
>>>(setq org-id-link-consider-parent-id t)
>>>(setq org-id-link-to-org-use-id t)
>>>
>>>Then, create a new empty Org file
>>>M-x org-store-link with create a top-level properties drawer with ID
>>>and store the link. However, that link will not be a simple ID link,
>>>but also have ::PROPERTIES search string, which is not expected.
>>
>> This is because it is trying to link to the current line of the file, which 
>> contains the text "PROPERTIES".  On main, with (setq 
>> org-id-link-to-org-use-id nil), you see the equivalent behaviour (a link to 
>> [[file:test.org:::PROPERTIES:]]) when point is before the first heading.  
>> So, this seems consistent with non-org-id links?
>
> No. Do note that my instructions start from _empty_ file. With
> org-id-link-to-org-use-id, PROPERTIES drawer is not created. This is
> different from what happens with your patch - it is unexpected in your
> patch that the search string is added for text that did not exist in the
> buffer previously.

I see.  Updated to get the search string first, before the possible properties 
draw appears.

To make this work I changed `org-link-precise-link-target': instead of 
accepting the RELATIVE-TO argument and rejecting unsuitable targets internally, 
it now sets a marker `org-link-precise-target-marker' showing where the target 
that was found is, so the caller can decide if the found target is suitable.  I 
copied the approach from `org-entry-property-inherited-from', hope that doesn't 
cause any other issues.

> That's a good catch.
> The fact that links stored via `org-store-link' cannot be open with
> default settings is not good.
> Also, your patch disregards this setting - it should not match
> non-headline search strings with the default value of
> `org-link-search-must-match-exact-headline'.

`org-link-search-must-match-exact-headline' affects `org-link-search', which is 
called by `org-id-open' -- so I think the behaviour for these org-id links 
should be the same as for other file links? Am I missing something?

Or, maybe you mean links that rely on 
`org-link-search-must-match-exact-headline' should not be stored.  That would 
seem reasonable, but also doesn't need to be part of these changes here?

> Probably, changing the default value of
> `org-link-search-must-match-exact-headline' to nil is due.

It seems like the behaviour below would be desirable, but doesn't currently 
exist with any setting of `org-link-search-must-match-exact-headline'?

(org-link-search "plain text")  -->  fuzzy search for all text
(org-link-search "*heading")-->  search only headings, optionally creating 
if missing

>> Subject: [PATCH 2/2] org-id.el: Extend links with search strings, inherit
>>  parent IDs
>
> I ran make test, and it looks like one test is failing with your patch:

Oops, fixed now I think.

> `org-context-in-file-links' is an obsolete name. Use
> `org-link-context-for-files'.
>
> Also, please add `org-id-link-use-context' to #+vindex.

Updated

> Please update the docstring of `org-store-link-functions' to specify
> that an argument is passed to :store functions.

Updated

>> -  (org-insert-heading nil t t)
>> +  ;; Find appropriate level for new heading
>> +  (let ((level (save-excursion
>> + (goto-char (point-min))
>> + (+ 1 (or (org-current-level) 0)
>
> This is fragile. You assume that `point-min' always contains a heading.
> That may or may not be the case - `org-link-search' may be called by
> third-party code that does not care about setting narrowing in certain
> ways.

I don't think it's a problem. (org-current-level) returns something suitable 
whether or not point-min contains a heading. Both the situations below seem 
reasonable choices for the level of the newly created heading at the end:

---start of narrowing---
Text
* H1
** H2
* A new level 1 heading is created at the end
---end of narrowing---

---start of narrowing---
* H1
** H2
** A new level 2 heading is created at the end
---end of narrowing---

(this is how it currently works, unless I'm missing something)

>> +(defun org-link-precise-link-target ( relative-to)
>> +  "Determine search string and description for storing a link.
>> +
>> +If a search string (see 'org-link-search') is found, return cons
>
> Quoting: `org-link-search'.

Fixed

>> +   (let* ((element (org-element-at-point))
>> +  (name (org-element-property :name element))
>> +  (heading (org-element-lineage element 'headline t))
>
> What about inlinetasks?

I added inlinetasks to the element types, so they are picked up the same as 
headlines now.

>> +  (custom-id (org-entry-get nil "CUSTOM_ID")))
>
> May as well pass HEADING as the first argument of `org-entry-get'. It
> will be slightly more efficient.

Ok

>> +(org-link--add-to-stored-links link desc)
>> +;; In org 

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

2024-01-29 Thread Samuel Wales
On 1/29/24, Ihor Radchenko  wrote:
> You mentioned this feature request in the past. It is not forgotten.

thank you.

> May you please elaborate what you want to add to the manual and where?

had been merely thinking mentioning non-brittleness for newcomers.

in handling links.

but MAYBE also org-id could be slightly more integrated in org, such
as not having to load the library.  9.6 info (info "(org) Handling
Links") has context of org buffers.  in future, i'd like to link to
org-id in non-org files.  (info "(org) Include Files") does not
mention whether org-id works.  could eliminate the need to specify
file.



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

2024-01-29 Thread Ihor Radchenko
Samuel Wales  writes:

> my experiene is that context, search, and file links typically break
> for me, as i change headers, refile, fix typos, change paths, etc.  so
> i stick with just org-id and, for export, custom id where possible.

That's why we have `org-id-link-to-org-use-id'.

> however, i would DEFINITELY also use org id link targets that are
> puttable in various locations [e.g. id markers].

You mentioned this feature request in the past. It is not forgotten.

> what i am pointing out is probably obvious, but might be worth
> pointing out in a sentence in manual, or for setting defaults?

May you please elaborate what you want to add to the manual and where?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2024-01-29 Thread Ihor Radchenko
"Rick Lupton"  writes:

> Thanks for trying it out.  Updated patches attached, comments below.

Thanks!

>> 3. Consider
>>(setq org-id-link-consider-parent-id t)
>>(setq org-id-link-to-org-use-id t)
>>
>>Then, create a new empty Org file
>>M-x org-store-link with create a top-level properties drawer with ID
>>and store the link. However, that link will not be a simple ID link,
>>but also have ::PROPERTIES search string, which is not expected.
>
> This is because it is trying to link to the current line of the file, which 
> contains the text "PROPERTIES".  On main, with (setq 
> org-id-link-to-org-use-id nil), you see the equivalent behaviour (a link to 
> [[file:test.org:::PROPERTIES:]]) when point is before the first heading.  So, 
> this seems consistent with non-org-id links?

No. Do note that my instructions start from _empty_ file. With
org-id-link-to-org-use-id, PROPERTIES drawer is not created. This is
different from what happens with your patch - it is unexpected in your
patch that the search string is added for text that did not exist in the
buffer previously.

> (these links don't actually work with the default value of
> `org-link-search-must-match-exact-headline', but I think that's a
> separate issue).

That's a good catch.
The fact that links stored via `org-store-link' cannot be open with
default settings is not good.
Also, your patch disregards this setting - it should not match
non-headline search strings with the default value of
`org-link-search-must-match-exact-headline'.

Probably, changing the default value of
`org-link-search-must-match-exact-headline' to nil is due.

> Subject: [PATCH 2/2] org-id.el: Extend links with search strings, inherit
>  parent IDs

I ran make test, and it looks like one test is failing with your patch:

1 unexpected results:
   FAILED  test-org-link/id-store-link-using-parent  ((should (equal '("id:abc" 
"H1") (test-ol-stored-link-with-text "* H1\n:PROPERTIES:\n:ID: abc\n:END:\n** 
H2\n" (org-id-store-link :form (equal ("id:abc" "H1") 
("id:88e0c8d7-90a6-4628-b35a-cea989e3561b" "H2")) :value nil :explanation 
(list-elt 0 (arrays-of-different-length 6 39 "id:abc" 
"id:88e0c8d7-90a6-4628-b35a-cea989e3561b" first-mismatch-at 3)))

> +  #+vindex: org-id-link-consider-parent-id
> +  When ~org-id-link-consider-parent-id~ is ~t~ (and
> +  ~org-context-in-file-links~ and ~org-id-link-use-context~ are both

`org-context-in-file-links' is an obsolete name. Use
`org-link-context-for-files'.

Also, please add `org-id-link-use-context' to #+vindex.

> + =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.
> +
> +Existing store functions will continue to work.

Please update the docstring of `org-store-link-functions' to specify
that an argument is passed to :store functions.

(org-link-parameters docstring says

:store

  Function responsible for storing the link.  See the function
  org-store-link-functions for a description of the expected
  arguments.
)

> -  (org-insert-heading nil t t)
> +  ;; Find appropriate level for new heading
> +  (let ((level (save-excursion
> + (goto-char (point-min))
> + (+ 1 (or (org-current-level) 0)

This is fragile. You assume that `point-min' always contains a heading.
That may or may not be the case - `org-link-search' may be called by
third-party code that does not care about setting narrowing in certain
ways.

It is more reliable to do something like (while (org-up-heading-safe)
...) to find the lowest-level ancestor.

> +(defun org-link-precise-link-target ( relative-to)
> +  "Determine search string and description for storing a link.
> +
> +If a search string (see 'org-link-search') is found, return cons

Quoting: `org-link-search'.

> +   (let* ((element (org-element-at-point))
> +  (name (org-element-property :name element))
> +  (heading (org-element-lineage element 'headline t))

What about inlinetasks?

> +  (custom-id (org-entry-get nil "CUSTOM_ID")))

May as well pass HEADING as the first argument of `org-entry-get'. It
will be slightly more efficient.

> +(org-link--add-to-stored-links link desc)
> +;; In org buffers, store an additional "human-readable" link
> +;; using custom id, if available.
> +(when (and (buffer-file-name (buffer-base-buffer))
> +   (derived-mode-p 'org-mode)
> +   (org-entry-get nil "CUSTOM_ID"))
> +  (setq link (concat "file:"
> + (abbreviate-file-name
> +  (buffer-file-name (buffer-base-buffer)))
> + "::#" (org-entry-get nil "CUSTOM_ID")))

This is fragile - you are relying upon the exact code used 

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

2024-01-28 Thread Samuel Wales
sounds like a lot of contribution.  i do not want to impede anything
anybody else wants, but want to point out my user experience over
years in case useful to anybody.

my experiene is that context, search, and file links typically break
for me, as i change headers, refile, fix typos, change paths, etc.  so
i stick with just org-id and, for export, custom id where possible.
however, i would DEFINITELY also use org id link targets that are
puttable in various locations [e.g. id markers].
what i am pointing out is probably obvious, but might be worth
pointing out in a sentence in manual, or for setting defaults?


On 1/28/24, Rick Lupton  wrote:
> Hi,
>
> Thanks for trying it out.  Updated patches attached, comments below.
>
> On Mon, 18 Dec 2023, at 12:27 PM, Ihor Radchenko wrote:
>> I played around with the patch a bit and found a couple of rough edges:
>>
>> 1. When I try to open a link to non-existing search target, like
>>, I get a query to create a new
>>heading. If I reply "yes", a new heading is created. However, the
>>heading is created at the end of the file and is always level 1,
>>regardless of the "some-id" parent context.
>>It would make more sense to create a new heading at the end of the
>>id:some-id subtree.
>
> Fixed in updated patches -- first patch adds generic new flexibility to
> `org-insert-heading', second patch uses it so new headings now added at
> correct level at the end of the id:sub-id subtree.
>
>> 2. Consider the following setting:
>>(setq org-id-link-consider-parent-id t)
>>(setq org-id-link-to-org-use-id
>> 'create-if-interactive-and-no-custom-id)
>>
>>Then, create the following Org file
>>
>> * Sub
>> * Parent here
>> ** This is test
>> :PROPERTIES:
>> :ID:   fe40252e-0527-44c1-a990-12498991f167
>> :END:
>>
>> *** Sub 
>> :PROPERTIES:
>> :CUSTOM_ID:   subid
>> :END:
>>
>>When you M-x org-store-link, the stored link has ::*Sub instead of
>>the expected ::#subid
>
> Updated so that search strings prefer custom-ids (::#subid) to headline
> matches (::*Sub).  This makes this example behave as you expect.
>
> The correct behaviour of org-store-link doesn't seem totally obvious to me
> about id vs custom-id links.  Currently org-store-link has special logic to
> store TWO links (one , one ) when a
> CUSTOM_ID is present. In the manual, it says:
>
>  If the headline has a ‘CUSTOM_ID’ property, store a link to this
>  custom ID.  In addition or alternatively, depending on the value of
>  ‘org-id-link-to-org-use-id’, create and/or use a globally unique
>  ‘ID’ property for the link(1).  So using this command in Org
>  buffers potentially creates two links: a human-readable link from
>  the custom ID, and one that is globally unique and works even if
>  the entry is moved from file to file.  The ‘ID’ property can be
>  either a UUID (default) or a timestamp, depending on
>  ‘org-id-method’.  Later, when inserting the link, you need to
>  decide which one to use.
>
> That refers to ID links specifically, but now, using the generic link store
> functions, there is only the possibility to store one link type, so it's not
> possible to neatly keep exactly the same behaviour (i.e. for ID links but
> not for other external link types).
>
> I think the intention of what's described in the manual is to distinguish
> "human-readable" vs "persistent id" links.  There could be other types of
> "persistent id" links apart from org-id links, such as mu4e: links to email
> message-ids.  Therefore I've updated org-store-link to simply store a
>  link as an additional option, whether or not the
> first matched link was an org-id link (this is the current behaviour) or
> another external link type (this is changed behaviour).
>
> Added a note to ORG-NEWS about this.
>
>> 3. Consider
>>(setq org-id-link-consider-parent-id t)
>>(setq org-id-link-to-org-use-id t)
>>
>>Then, create a new empty Org file
>>M-x org-store-link with create a top-level properties drawer with ID
>>and store the link. However, that link will not be a simple ID link,
>>but also have ::PROPERTIES search string, which is not expected.
>
> This is because it is trying to link to the current line of the file, which
> contains the text "PROPERTIES".  On main, with (setq
> org-id-link-to-org-use-id nil), you see the equivalent behaviour (a link to
> [[file:test.org:::PROPERTIES:]]) when point is before the first heading.
> So, this seems consistent with non-org-id links?
>
> (these links don't actually work with the default value of
> `org-link-search-must-match-exact-headline', but I think that's a separate
> issue).
>
>>> +  #+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 

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

2024-01-28 Thread Rick Lupton
Hi,

Thanks for trying it out.  Updated patches attached, comments below.

On Mon, 18 Dec 2023, at 12:27 PM, Ihor Radchenko wrote:
> I played around with the patch a bit and found a couple of rough edges:
>
> 1. When I try to open a link to non-existing search target, like
>, I get a query to create a new
>heading. If I reply "yes", a new heading is created. However, the
>heading is created at the end of the file and is always level 1,
>regardless of the "some-id" parent context.
>It would make more sense to create a new heading at the end of the
>id:some-id subtree.

Fixed in updated patches -- first patch adds generic new flexibility to 
`org-insert-heading', second patch uses it so new headings now added at correct 
level at the end of the id:sub-id subtree.

> 2. Consider the following setting:
>(setq org-id-link-consider-parent-id t)
>(setq org-id-link-to-org-use-id 'create-if-interactive-and-no-custom-id)
>
>Then, create the following Org file
>
> * Sub
> * Parent here
> ** This is test
> :PROPERTIES:
> :ID:   fe40252e-0527-44c1-a990-12498991f167
> :END:
>
> *** Sub 
> :PROPERTIES:
> :CUSTOM_ID:   subid
> :END:
>
>When you M-x org-store-link, the stored link has ::*Sub instead of
>the expected ::#subid

Updated so that search strings prefer custom-ids (::#subid) to headline matches 
(::*Sub).  This makes this example behave as you expect.

The correct behaviour of org-store-link doesn't seem totally obvious to me 
about id vs custom-id links.  Currently org-store-link has special logic to 
store TWO links (one , one ) when a CUSTOM_ID 
is present. In the manual, it says:

 If the headline has a ‘CUSTOM_ID’ property, store a link to this
 custom ID.  In addition or alternatively, depending on the value of
 ‘org-id-link-to-org-use-id’, create and/or use a globally unique
 ‘ID’ property for the link(1).  So using this command in Org
 buffers potentially creates two links: a human-readable link from
 the custom ID, and one that is globally unique and works even if
 the entry is moved from file to file.  The ‘ID’ property can be
 either a UUID (default) or a timestamp, depending on
 ‘org-id-method’.  Later, when inserting the link, you need to
 decide which one to use.

That refers to ID links specifically, but now, using the generic link store 
functions, there is only the possibility to store one link type, so it's not 
possible to neatly keep exactly the same behaviour (i.e. for ID links but not 
for other external link types).

I think the intention of what's described in the manual is to distinguish 
"human-readable" vs "persistent id" links.  There could be other types of 
"persistent id" links apart from org-id links, such as mu4e: links to email 
message-ids.  Therefore I've updated org-store-link to simply store a 
 link as an additional option, whether or not the 
first matched link was an org-id link (this is the current behaviour) or 
another external link type (this is changed behaviour).

Added a note to ORG-NEWS about this.

> 3. Consider
>(setq org-id-link-consider-parent-id t)
>(setq org-id-link-to-org-use-id t)
>
>Then, create a new empty Org file
>M-x org-store-link with create a top-level properties drawer with ID
>and store the link. However, that link will not be a simple ID link,
>but also have ::PROPERTIES search string, which is not expected.

This is because it is trying to link to the current line of the file, which 
contains the text "PROPERTIES".  On main, with (setq org-id-link-to-org-use-id 
nil), you see the equivalent behaviour (a link to 
[[file:test.org:::PROPERTIES:]]) when point is before the first heading.  So, 
this seems consistent with non-org-id links?

(these links don't actually work with the default value of 
`org-link-search-must-match-exact-headline', but I think that's a separate 
issue).

>> +  #+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.
>
> It would be nice to add an example, similar to what you did in the docstring.

Added.

>
>> -(defun org-man-store-link ()
>> +(defun org-man-store-link ( _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.
>> @@ -21312,13 +21324,15 @@ A review of =ol-man.el=:
>
> Please, update the actual built-in :store functions in lisp/ol-*.el to
> handle the new optional argument as well.

Updated.

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

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

2024-01-03 Thread Ihor Radchenko
"Rick Lupton"  writes:

> Thanks for the comments. On this point, I'd like to modify 
> `org-insert-heading' to allow for choosing the level of the newly inserted 
> heading, but first wanted to check if you have a preference for how to change 
> it.
>
>
> I think it would be simplest to change the current:
>
> (defun org-insert-heading ( arg invisible-ok top)
>   "...When optional argument TOP is non-nil, insert a level 1 heading, 
> unconditionally."
>
> to:
>
> (defun org-insert-heading ( arg invisible-ok level)
>   "...When optional argument LEVEL is a number, insert a heading at that 
> level.  For backwards compatibility, when LEVEL is non-nil but not a number, 
> insert a level-1 heading."
>
> but that is not totally backwards compatible -- is that ok?

I think that it is OK. I very much doubt that anyone at all uses
numerical value for TOP in the existing `org-insert-heading' calls from
Elisp. And adding yet another extra optional argument is not justified
in this particular case.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2024-01-02 Thread Rick Lupton
On Mon, 18 Dec 2023, at 12:27 PM, Ihor Radchenko wrote:
> I played around with the patch a bit and found a couple of rough edges:
>
> 1. When I try to open a link to non-existing search target, like
>, I get a query to create a new
>heading. If I reply "yes", a new heading is created. However, the
>heading is created at the end of the file and is always level 1,
>regardless of the "some-id" parent context.
>It would make more sense to create a new heading at the end of the
>id:some-id subtree.

Thanks for the comments. On this point, I'd like to modify `org-insert-heading' 
to allow for choosing the level of the newly inserted heading, but first wanted 
to check if you have a preference for how to change it.


I think it would be simplest to change the current:

(defun org-insert-heading ( arg invisible-ok top)
  "...When optional argument TOP is non-nil, insert a level 1 heading, 
unconditionally."

to:

(defun org-insert-heading ( arg invisible-ok level)
  "...When optional argument LEVEL is a number, insert a heading at that level. 
 For backwards compatibility, when LEVEL is non-nil but not a number, insert a 
level-1 heading."

but that is not totally backwards compatible -- is that ok?


If it should be completely backwards compatible, alternatively could add an 
additional optional argument:

(defun org-insert-heading ( arg invisible-ok top top-level)
  "...When optional argument TOP is non-nil, insert a top-level heading, 
unconditionally.  When TOP-LEVEL is non-nil, use that level, otherwise level 1."


Alternatively I could preserve the intention of TOP but add a special value to 
change what "top-level" means, so the docstring would become something like 
this:

"When optional argument TOP is non-nil, insert a top-level
heading, unconditionally.  Specifically, when TOP is `relative',
\"top-level\" means one level deeper than the outline level at
minimum point position (respecting any narrowing of the buffer).
Otherwise, \"top-level\" means level 1."

(the motivation for this is that when the buffer is narrowed to the subtree 
with the matching ID, the new heading will be created at the appropriate level).


Best
Rick



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

2023-12-18 Thread Ihor Radchenko
"Rick Lupton"  writes:

> Please find attached updated patch which I think addresses all the points 
> discussed.  Let me know if you see any further changes needed.

Thanks!

I played around with the patch a bit and found a couple of rough edges:

1. When I try to open a link to non-existing search target, like
   , I get a query to create a new
   heading. If I reply "yes", a new heading is created. However, the
   heading is created at the end of the file and is always level 1,
   regardless of the "some-id" parent context.
   It would make more sense to create a new heading at the end of the
   id:some-id subtree.

2. Consider the following setting:
   (setq org-id-link-consider-parent-id t)
   (setq org-id-link-to-org-use-id 'create-if-interactive-and-no-custom-id)

   Then, create the following Org file

* Sub
* Parent here
** This is test
:PROPERTIES:
:ID:   fe40252e-0527-44c1-a990-12498991f167
:END:

*** Sub 
:PROPERTIES:
:CUSTOM_ID:   subid
:END:

   When you M-x org-store-link, the stored link has ::*Sub instead of
   the expected ::#subid

3. Consider
   (setq org-id-link-consider-parent-id t)
   (setq org-id-link-to-org-use-id t)

   Then, create a new empty Org file
   M-x org-store-link with create a top-level properties drawer with ID
   and store the link. However, that link will not be a simple ID link,
   but also have ::PROPERTIES search string, which is not expected.

More inline comments below.

> +  #+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.

It would be nice to add an example, similar to what you did in the docstring.

> -(defun org-man-store-link ()
> +(defun org-man-store-link ( _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.
> @@ -21312,13 +21324,15 @@ A review of =ol-man.el=:

Please, update the actual built-in :store functions in lisp/ol-*.el to
handle the new optional argument as well.

> + =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.

Please also explain that the existing functions are not broken.

> +*** ~org-id-store-link~ now adds search strings for precise link targets
> +
> +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~.

universal argument.
There are several possible prefix arguments in `org-store-link', but
only C-u (universal argument) will give the described effect.
Also, won't the behavior be _toggled_ by the 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).

Please add an org-lint checker that warns about such IDs and mention
this checker in the above.
Also, this paragraph belongs to "Breaking changes", not "new and changed
options".

> +*** 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.

Please, provide an example here as well.

> +(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 when a link has been stored in `org-link-store-props'."

Please document INTERACTIVE? argument in the docstring.

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

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

2023-12-17 Thread Rick Lupton
Please find attached updated patch which I think addresses all the points 
discussed.  Let me know if you see any further changes needed.

Thanks,
Rick


0001-org-id.el-Extend-links-with-search-strings-inherit-v2.patch
Description: Binary data