[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>
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Ihor Radchenko, 2024/02/01
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines,
Ihor Radchenko <=
- [PATCH] lisp/ol.el: Improve docstring, Rick Lupton, 2024/02/08
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Rick Lupton, 2024/02/08
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Ihor Radchenko, 2024/02/08
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Rick Lupton, 2024/02/08
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Ihor Radchenko, 2024/02/09
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Rick Lupton, 2024/02/09
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Ihor Radchenko, 2024/02/09
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Bastien Guerry, 2024/02/24
- Re: [PATCH v2] org-id: allow using parent's existing id in links to headlines, Ihor Radchenko, 2024/02/24