emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ihor Radchenko
Subject: Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines
Date: Sat, 03 Feb 2024 13:10:17 +0000

"Rick Lupton" <mail@ricklupton.name> 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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]