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

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

> Thanks for the example and explanation. Yes that does make sense, mostly. I 
> assume this would look like this in org-store-link:
>
> (let ((org-link-context-for-files (org-xor org-link-context-for-files (equal 
> arg '(4
> (...call store link functions...))

Yes.

> The meaning of `org-link-context-for-files' is then shifting from being 
> "should file: links include search strings (and how much should be included 
> when the region is active)" from "should any link that supports search 
> strings include them (and how much should be included when the region is 
> active)". Is it necessary to rename it to reflect this? (e.g. to 
> `org-link-use-context' or similar).

I do not think so - with your addition, we are still linking to files.
May simply update the docstring for `org-link-context-for-files' and
`org-store-link'.

> It's also then less clear what the role of `org-id-link-use-context' is and 
> how it interacts with `org-link-context-for-files'. I had included 
> `org-id-link-use-context' to give a way to opt out of the new behaviour (i.e. 
> using the update discussed above, a search string is added if (and 
> org-link-context-for-files org-id-link-use-context) ). But perhaps this is 
> also unnecessarily complicated, and `org-id-link-use-context' could be 
> removed again completely?

I do not think so - it is important to keep an option for users to
return to previous behaviour.

We might, in theory, modify `org-link-context-for-files' to allow
per-link type customization (then, people could set things back to
"context just for file: links"), but it would be more complicated IMHO.

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



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

2023-12-15 Thread Rick Lupton
On Fri, 15 Dec 2023, at 12:55 PM, Ihor Radchenko wrote:
> No, it is generally not safe. For a different reason.
>
> Let me illustrate with an example:
>
> ...
>
> Conclusion: It is unsafe to use `current-prefix-arg' value. We need to
> pass this information some other way.
>
> The way I proposed is actually not any special for ID links. What I
> meant it to let-bind `org-link-context-for-files' around the whole call
> to `org-store-link-functions', so that the custom :store functions will
> get access to the adjusted value of `org-link-context-for-files'.
> Does this explanation make more sense?

Thanks for the example and explanation. Yes that does make sense, mostly. I 
assume this would look like this in org-store-link:

(let ((org-link-context-for-files (org-xor org-link-context-for-files (equal 
arg '(4
(...call store link functions...))

The meaning of `org-link-context-for-files' is then shifting from being "should 
file: links include search strings (and how much should be included when the 
region is active)" from "should any link that supports search strings include 
them (and how much should be included when the region is active)". Is it 
necessary to rename it to reflect this? (e.g. to `org-link-use-context' or 
similar).

It's also then less clear what the role of `org-id-link-use-context' is and how 
it interacts with `org-link-context-for-files'. I had included 
`org-id-link-use-context' to give a way to opt out of the new behaviour (i.e. 
using the update discussed above, a search string is added if (and 
org-link-context-for-files org-id-link-use-context) ). But perhaps this is also 
unnecessarily complicated, and `org-id-link-use-context' could be removed again 
completely?

> I will update the docstring of
> `org-link-search' to explicitly specify that it is searching within the
> accessible portion of the buffer and update the callers to account for
> this.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=89164e605
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5c543cd9d
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=cb71bde7c
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=63ef7b924
>
> But does your code do narrowing? I did not notice it.

Not in the patch I sent, I added it later after you pointed this out.

I'll send an updated patch next.

Thanks,
Rick



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

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

>>> +(defcustom org-id-link-use-context t
>> ...
>> It does not look like integer value is respected in the patch.
>
> You're right. Do you have a preference between
>
> (a) sticking to this docstring, which creates the possibility of using 
> different numbers of lines for id: and file: links' context, and makes the 
> code slightly more complicated, but keeps the meaning of 
> `org-link-context-for-files' specifically `for files'; or
>
> (b) Always use `org-link-context-for-files' to set the number of lines of 
> context used for all links; `org-id-link-use-context' is just a boolean. The 
> code is simpler.
>
> ?

I think that (b) makes sense. There is no reason to make the
customization yet more granular and complex when there is no clear need.
We can always do it later, if necessary, anyway.

>>> -  (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.
>
> Good point; I think the answer is to force INHERIT to t or nil, rather than 
> documenting and continuing to accept 'selective (when INHERIT is used, it 
> should definitely take effect).

Agree.

>>> +  ;; 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'.
>
> Now that `org-id-store-link' is called via :store link property, 
> `org-store-link` does not have special logic for org-id, which I thought was 
> an improvement, so it would be a step backwards to add in special logic for 
> `org-id-link-use-context'?
>
> Instead, I think this logic could be in `org-id-store-link-maybe' as above. 
> That is, it is safe to take account of `current-prefix-arg' within a link 
> :store function, and assume it represents prefix args as used with 
> `org-store-link'? 

No, it is generally not safe. For a different reason.

Let me illustrate with an example:

(defun yant/test2 ()
  (message "current-prefix-arg: %S" current-prefix-arg))

(defun yant/test (arg)
  (interactive "P")
  (yant/test2))

When you call M-x yant/test, you will see "current-prefix-arg: nil".
However, when you call C-u M-x yant/test, you will see
"current-prefix-arg: (4)".

Similar logic applies to the non-interactive calls to `org-store-link'.
If some Elisp code implements a command like

(defun yant/my-command (arg)
  (interactive "P")
  
  (org-store-link nil))

then, `org-store-link' may call `org-id-store-link-maybe' and
`org-id-store-link-maybe' will still "see" the top-level prefix argument
passed to `yant/my-command' - the prefix argument that has nothing at
all to do with prefix arguments of `org-store-link'.

Conclusion: It is unsafe to use `current-prefix-arg' value. We need to
pass this information some other way.

The way I proposed is actually not any special for ID links. What I
meant it to let-bind `org-link-context-for-files' around the whole call
to `org-store-link-functions', so that the custom :store functions will
get access to the adjusted value of `org-link-context-for-files'.
Does this explanation make more sense?

>>> +(pcase (org-link-precise-link-target id-location)
>>
>> Why not passing the RELATIVE-TO argument?
>
> The `id-location' is the RELATIVE-TO argument. Or do I misunderstand you?

I just did not notice ID-LOCATION :facepalm:

>>> +(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.
>
> Thanks, yes. It looks to me (from the code and some testing) that narrowing 
> to the target heading first before calling `org-link-search' does the right 
> thing. Was there a particular reason you thought `org-link-search' would need 
> to be changed?

Because a lot of the code (except some part `org-link-open') assumes
that `org-link-search' searches the whole buffer, not just the narrowed
part. But that's not your problem - I will update the docstring of
`org-link-search' to explicitly specify that it is searching within the
accessible portion of the buffer and update the callers to account for
this.

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

2023-12-14 Thread Rick Lupton
Dear Ihor,

Thanks for taking a look at the patch and for your feedback.

Re various docstrings, documenting new optional argument -- I will try to 
address these.

Other comments/questions below.

On Sun, 10 Dec 2023, at 1:35 PM, Ihor Radchenko wrote:
> "This change..." part sounds like a commit message, not a NEWS item.

I think there are lots of other examples that are written like this in ORG-NEWS 
(but I agree my sentence was unnecessary and have removed it).

>> +(defcustom org-id-link-use-context t
>> +  "Non-nil means org-id links from `org-id-store-link' contain context.
>> +\\
>> +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.

You're right. Do you have a preference between

(a) sticking to this docstring, which creates the possibility of using 
different numbers of lines for id: and file: links' context, and makes the code 
slightly more complicated, but keeps the meaning of 
`org-link-context-for-files' specifically `for files'; or

(b) Always use `org-link-context-for-files' to set the number of lines of 
context used for all links; `org-id-link-use-context' is just a boolean. The 
code is simpler.

?

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

Good point; I think the answer is to force INHERIT to t or nil, rather than 
documenting and continuing to accept 'selective (when INHERIT is used, it 
should definitely take effect).

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

Oops, yes -- but in fact this argument is only needed on 
`org-id-store-link-maybe' (as below), so I can remove it again here.

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

Ok, yes that makes sense.

>> +  ;; 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'.

Now that `org-id-store-link' is called via :store link property, 
`org-store-link` does not have special logic for org-id, which I thought was an 
improvement, so it would be a step backwards to add in special logic for 
`org-id-link-use-context'?

Instead, I think this logic could be in `org-id-store-link-maybe' as above. 
That is, it is safe to take account of `current-prefix-arg' within a link 
:store function, and assume it represents prefix args as used with 
`org-store-link'? 

>
>> +(pcase (org-link-precise-link-target id-location)
>
> Why not passing the RELATIVE-TO argument?

The `id-location' is the RELATIVE-TO argument. Or do I misunderstand you?

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

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

2023-12-10 Thread Ihor Radchenko
"Rick Lupton"  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.

>  
> +(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.
> +\\
> +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 ( force)
> +(defun org-id-get-create ( force inherit)
>...
>  ;;;###autoload
> -(defun org-id-get ( epom create prefix)
> +(defun org-id-get ( 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 

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

2023-12-04 Thread Rick Lupton
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



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

2023-11-19 Thread Rick Lupton
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


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


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

2023-11-10 Thread Ihor Radchenko
"Rick Lupton"  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 .
Support Org development at ,
or support my work at 



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

2023-11-09 Thread Rick Lupton
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



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

2023-11-05 Thread Ihor Radchenko
"Rick Lupton"  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 .
Support Org development at ,
or support my work at 



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

2023-11-04 Thread Rick Lupton
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



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

2023-07-29 Thread Ihor Radchenko
"Rick Lupton"  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 .
Support Org development at ,
or support my work at 



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-28 Thread Rick Lupton
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  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+ovd0ncwzztpit5t7wvsblbgllxzmpub5tgq3gshsg...@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 .
> Support Org development at ,
> or support my work at 



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

2023-07-28 Thread Rick Lupton
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 , we should better also
> provide  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



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 Thread Ihor Radchenko
Samuel Wales  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+ovd0ncwzztpit5t7wvsblbgllxzmpub5tgq3gshsg...@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 .
Support Org development at ,
or support my work at 



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

2023-07-26 Thread Samuel Wales
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  wrote:
> Max Nikulin  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 , we should better also
> provide  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 .
> Support Org development at ,
> or support my work at 
>
>


-- 
The Kafka Pandemic

A blog about science, health, human rights, and misopathy:
https://thekafkapandemic.blogspot.com



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

2023-07-26 Thread Ihor Radchenko
Max Nikulin  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 , we should better also
provide  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 .
Support Org development at ,
or support my work at 



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

2023-07-25 Thread Max Nikulin

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.




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

2023-07-25 Thread Ihor Radchenko
"Rick Lupton"  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 ( 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 .
Support Org development at ,
or support my work at 



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

2023-07-24 Thread Rick Lupton
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
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

0001-lisp-org-id.el-Allow-using-a-parent-s-existing-id.patch
Description: Binary data