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 (&optional 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 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 to store > file:...#CUSTOM-ID link. Instead, please refactor the function to re-use > that code. Ok >> + (id-location (or (and org-entry-property-inherited-from >> + (marker-position org-entry-property-inherited-from)) >> + (save-excursion (org-back-to-heading-or-point-min) (point)))) >> (case-fold-search nil) >> (desc (save-excursion >> - (org-back-to-heading-or-point-min t) >> + (goto-char id-location) > > You are calling `org-back-to-heading-or-point-min' without optional > argument INVISIBLE-OK. This looks like an oversight. Fixed