org-transclusion icon indicating copy to clipboard operation
org-transclusion copied to clipboard

Transclusion of a heading within an org file won't work for ID-type links

Open axbweiss opened this issue 1 year ago • 20 comments

For any org file containing an ID, the content of headings referenced by file type links can be transcluded, but this doesn't happen for ID-type links.

(Following the link itself works, so the link isn't broken.)


[[file:somenode.org::*Some heading][Some heading]]

Is transcluded with no problems

[[id:4a3fc3b3-caef-47d7-8e2d-ee0d246b63de::*Some heading][Some heading]]

Causes the error

No content found with "id:4a3f...*Some heading". Check the link at point...

I am using Doom (on Linux), in case it could be a related issue.

axbweiss avatar Apr 16 '24 23:04 axbweiss

Can you try ID without the “::*Sime heading” part, please?

[[id:4a3fc3b3-caef-47d7-8e2d-ee0d246b63de][Some heading]]

nobiot avatar Apr 17 '24 05:04 nobiot

Can you try ID without the “::*Sime heading” part, please?

[[id:4a3fc3b3-caef-47d7-8e2d-ee0d246b63de][Some heading]]

Ah, I forgot to specify that ID-type links are transcluded just fine! There are only errors when the "::*Some heading" part is added.

(The reason I'd rather not give the heading I want to reference an ID is that I'm using org roam and I use headings within a node to organize notes on the topic. I'd rather not have headings within nodes become nodes themselves)

axbweiss avatar Apr 17 '24 14:04 axbweiss

@axbweiss

This is a custom feature that Doom Emacs has added, and has yet to be incorporated in the upstream Org.

I am not able to test this with Doom-specific syntax because I am not a Doom user. But perhaps you can add a customizing by using the same technique discussed in #160. I am not against adding it to the code base if this can be done cleanly. Will you be willing to do try?

nobiot avatar Apr 17 '24 18:04 nobiot

I did not know of this functionality before -- it is very helpful indeed I do not have to worry about creating separate id's for sub headings -- the procedure is rather simple -- but it modifies the open-id functions only, I will look into extending it for org-transclusion - seems to be rather simple from the example cited, we need to on the fly convert id to file type link I think. I will get back if I manage to write it. I need this feature too.

(defun org-id-open--support-search (fn link &optional arg)
  "Support ::SEARCH syntax for id: links."
  ;; Save match data to prevent modifications by string matching functions.
  (save-match-data
    ;; Split the link at the '::' delimiter.
    (let* ((parts (split-string link "::"))  ; Split the link into parts.
           (id (car parts))                  ; Extract the ID part.
           (search (cadr parts)))            ; Extract the search part.
      ;; Execute the original function and perform search operations.
      (progn (funcall fn id arg)
        ;; Conditionally execute additional actions based on the search part.
             (cond ((null search))                                   ; Do nothing if search part is null.
		   ((string-match-p "\\`[0-9]+\\'" search)           ; If search part is a number.
		    (forward-line (string-to-number search)))        ; Move N lines after the ID.
		   ((string-match "^/\\([^/]+\\)/$" search)          ; If search part is a regex match.
		    (let ((match (match-string 1 search)))           ; Extract the match string.
                      (save-excursion (org-link-search search))      ; Search for the match.
                      (when (re-search-forward match)                ; When match is found.
			(goto-char (match-beginning 0)))))           ; Move point to the match.
		   ((org-link-search search)))))))                   ; Otherwise, perform org-link-search.


;; Add Advice around id open functions
(advice-add 'org-id-open :around #'org-id-open--support-search)
(advice-add 'org-roam-id-open :around #'org-id-open--support-search)

This is the code that non-doom users can simply run to get the functionality. Its extremely simple.

Update

  • Fixed a case where ::number did not resolve properly

akashpal-21 avatar Apr 17 '24 21:04 akashpal-21

I was previously treating the link as a string -- this was a very costly mistake as it took me a lot of time to learn from trial and error how to properly parse propertized links.

This will work -- tested it on my end

(defun custom-org-transclusion-add-org-id (link plist)
  (when (string= "id" (org-element-property :type link))
    (let* ((custom-org-id (org-element-property :path link))
           (parts (split-string custom-org-id "::"))  ; Split the link into parts.
           (id (car parts))                           ; Extract the ID part.
           (search (cadr parts))                      ; Extract the search part.
           (file-path (condition-case nil
                          (org-roam-node-file (org-roam-node-from-id id))
                        (error nil)))                   
           (new-link (if file-path
                         (with-temp-buffer
                           (insert "[[file:")
                           (insert file-path)
                           (when search
                             (insert "::")
                             (insert search))
                           (insert "]]")
                           (beginning-of-buffer)
                           (org-element-link-parser))
                       nil))                               ; Set new-link to nil if file path couldn't be resolved.
           )		     
      (if new-link
          (org-transclusion-add-org-file new-link plist)
        (format "No transclusion done for this ID. File path couldn't be resolved"))
      )))

(with-eval-after-load 'org-transclusion
  (add-to-list 'org-transclusion-add-functions 'custom-org-transclusion-add-org-id)
  (setq org-transclusion-add-functions
      (remove 'org-transclusion-add-org-id org-transclusion-add-functions)))

removing the default org-transclusion function for org-id is not strictly necessary as our custom function is added to the front of the list and takes precedence -- but they try to do the same thing, so redundant.

  • Update: Fixed a case where #'org-element-link-parser was not correctly parsing links when search term had space in it.
  • Introduced better error handing and graceful quit on error.

akashpal-21 avatar Apr 18 '24 04:04 akashpal-21

Thank you @akashpal-21! I can confirm that the function is added to the org-transclusion-add-functions variable and that the original ID function was removed from it. The custom function works for ID-type links without any headings referenced, i.e. [[id:4a3fc3b3-caef-47d7-8e2d-ee0d246b63de][File]]. However, when attempting [[id:4a3fc3b3-caef-47d7-8e2d-ee0d246b63de::*Some heading][Some heading]] there is a new error: "No match for fuzzy expression:*Some"

I'll look into this these days; it's probably Doom-related, since it works on your end.

axbweiss avatar Apr 18 '24 12:04 axbweiss

@axbweiss @akashpal-21

An alternative is to use org-roam-db-extra-links-exclude-keys. You can add your own custom property, eg "ROAM_EXCLUDE" to this list. You then add "ROAM_EXCLUDE" to the node with a value "t" (or whatever).

This is probably easier given the error you are experiencing, @axbweiss .

It works on my end.

image

image

nobiot avatar Apr 18 '24 15:04 nobiot

Thank you @akashpal-21! I can confirm that the function is added to the org-transclusion-add-functions variable and that the original ID function was removed from it. The custom function works for ID-type links without any headings referenced, i.e. [[id:4a3fc3b3-caef-47d7-8e2d-ee0d246b63de][File]]. However, when attempting [[id:4a3fc3b3-caef-47d7-8e2d-ee0d246b63de::*Some heading][Some heading]] there is a new error: "No match for fuzzy expression:*Some"

I'll look into this these days; it's probably Doom-related, since it works on your end.

The function is a simple translation layer that when detects and 'ID' link is being parsed converts it to a file link of equivalent type, so it doesn't rely on the fuzzy parsing that doom introduces -- but rather on the fuzzy parsing of file links that emacs supports by default.

Can you confirm that you are using the latest version of the function -- I fixed the problem when the translation failed if it had a space in it -- i did a silly mistake when parsing the link earlier.

The error you indicate lets me believe its the old function -- that is failing to pick up on the "heading" of "some heading"

We can debug this more if problem persists, fix will be trivial whatever is happening.

Let know when you have free time, Best.

akashpal-21 avatar Apr 19 '24 00:04 akashpal-21

Can you confirm that you are using the latest version of the function -- I fixed the problem when the translation failed if it had a space in it -- i did a silly mistake when parsing the link earlier.

@akashpal-21 Yes, I was using that function and it actually does work!! Thank you so much for your help! I think it'd be a great extension for Doom users, or anyone who has search syntax set up for ID links.

#52 was the actual issue; while testing the function, I'd had the source for the transclusion open in the same buffer as well. It still slips my mind to close source buffers before transcluding.

An alternative is to use org-roam-db-extra-links-exclude-keys.

Thank you @nobiot! I'll be sure to remember that functionality for the future.

axbweiss avatar Apr 20 '24 11:04 axbweiss

I've also added your workaround from #52 to my config, nobiot:

(advice-remove 'org-link-search '+org--recenter-after-follow-link-a)

It works. Thank you for this as well!

axbweiss avatar Apr 20 '24 11:04 axbweiss

@axbweiss Will you close the issue, or do you prefer to keep it open?

nobiot avatar Apr 20 '24 12:04 nobiot

@nobiot I opted to keep it open in case @akashpal-21 would like to make a merge request to add their function as an extension. If they'd rather not, I'll close the issue but might still make a request myself (crediting them, of course)

axbweiss avatar Apr 20 '24 12:04 axbweiss

We could add something like this to the Manual instead of creating an extension -- we need to circumvent the id resolution, creating an extension could be an overkill in my opinion and would require complications which we should avoid.

Also we should use a more generalised file-path resolution than taking for granted that (featurep 'org-roam) is t

a more simplified but generalised equivalent would be the following

(defun org-transclusion-add-org-id--tofile (link &rest args)
  (when (string= "id" (org-element-property :type link))
    (let* ((raw-link (org-element-property :path link))
	   (parts (split-string raw-link "::"))       ; Split the link into parts.
	   (id (car parts))                           ; Extract the ID part.
	   (search (cadr parts))                      ; Extract the search part.
	   (file-path (ignore-errors
			(car (org-id-find id))))      ; define file path resolution from id
	   (new-link
	    (with-temp-buffer
	      (insert (format "[[file:%s" file-path))
	      (when search
		(insert "::" search))
	      (insert "]]")
	      (beginning-of-line)
	      (org-element-link-parser))))
      (org-transclusion-add-org-file new-link args))))

(add-to-list 'org-transclusion-add-functions 'org-transclusion-add-org-id--tofile)
(setq org-transclusion-add-functions
      (remove 'org-transclusion-add-org-id org-transclusion-add-functions))

Error handling do not need to be handled by this function -- since #'org-transclusion-add-org-file already does that for us

akashpal-21 avatar Apr 21 '24 07:04 akashpal-21

PROBLEM

One problem with current implementation is that - this just borks links to headline nodes (contra file nodes) We need to cleverly circumvent this problem -- the issue we face is that [[file:<file-path>::line-number]] is not supported and this is a known issue -- so we need to somehow identify the node position within a file

My solution is to get the headline name if we are dealing with a headline node within a file -- but let ::search get priority if provided explicitly by user

(defun org-transclusion-add-org-id--tofile (link &rest args)
  "Translate ID type links to FILE type links
to support <id>::*headline in org-transclusion"
  
  (when (string= "id" (org-element-property :type link))
    (let* ((raw-link (org-element-property :path link))
	   (parts (split-string raw-link "::"))      
	   (id (car parts))                          
	   (search (cadr parts))

	   (org-id-info (ignore-errors (org-id-find id)))
	   (file-path (car org-id-info))
	   (node-pos (cdr org-id-info))
	   (headline
	    (when (> node-pos 1)
	      (with-current-buffer (find-file-noselect file-path)
	      (goto-char node-pos)
	      (org-get-heading t t t t))))
	   
	   (new-link
	    (with-temp-buffer
	      (insert (format "[[file:%s" file-path))
	      (if search
		  (insert "::" search)
		(when headline
		  (insert "::*" headline)))
	      (insert "]]")
	      (beginning-of-line)
	      (org-element-link-parser))))
      
      (org-transclusion-add-org-file new-link args))))

This implementation works for all cases I have tested.

Update

  • Fixed a case where file-nodes failed to resolve - aargh !
  • Fixed a case where the function was querying the database twice -- redundant and inefficient -- reuse available information.

Problems for the Future/ Known Limitations

  • How to resolve the case if there are two headlines with the same name in the file ?

Currently I do not know how to solve this -- one way is to use custom-id for conflicting headline names if they come up and link to those.

akashpal-21 avatar Apr 21 '24 17:04 akashpal-21

If we add support for line numbers for FILE Type links as documented in #241 we can use a better way

(defun org-transclusion-add-org-id--tofile (link &rest args)
  "Translate ID type links to FILE type links
to support <id>::*headline in org-transclusion"
  
  (when (string= "id" (org-element-property :type link))
    (let* ((raw-link (org-element-property :path link))
	   (parts (split-string raw-link "::"))      
	   (id (car parts))                          
	   (search (cadr parts))

	   (org-id-info (ignore-errors (org-id-find id)))
	   (file-path (car org-id-info))
	   (node-pos (cdr org-id-info))
	   (line-pos
	    (when (> node-pos 1)
	      (with-current-buffer (find-file-noselect file-path)
		(goto-char node-pos)
		(let ((line-info (what-line)))
		  (string-match "Line \\([0-9]+\\)" line-info)
		  (match-string 1 line-info))
		)))
	   
	   (new-link
	    (with-temp-buffer
	      (insert (format "[[file:%s" file-path))
	      (if search
		  (insert "::" search)
		(when line-pos
		  (insert "::" line-pos)))
	      (insert "]]")
	      (beginning-of-line)
	      (org-element-link-parser))))
      
      (org-transclusion-add-org-file new-link args))))

akashpal-21 avatar Apr 21 '24 19:04 akashpal-21

This is also org-mode feature not doom emacs only (at least from org-mode 9.7 version). I came to this issue because I also tried the tranclusion with the link [[id:<org-mode ID>::*<org-mode heading>]] without success.

For reference org-mode 9.7:

New option org-id-link-consider-parent-id to allow id: links to parent headlines

Storing a link with point at "Child 1" will produce a link <id:abc::*Child 1>, which precisely links to the "Child 1" headline even though it does not have its own ID.

Also the terse description in the manual:

id id:B7423F4D-2E8A-471B-8810-C40F074717E9 id:B7423F4D-2E8A-471B-8810-C40F074717E9::*task (headline search)

jarofromel avatar Dec 20 '24 20:12 jarofromel

This is also org-mode feature not doom emacs only (at least from org-mode 9.7 version).

I realize Org 9.7 has added this new feature at some point. 9.7 was released in June 2024; the original conversation happened 1-2 months prior to it in April 2024.

I am not certain when I can work to adjust org-transclusion. I'd love support here.

nobiot avatar Dec 20 '24 20:12 nobiot

Present scenario

  1. org-id :: search is still work in progress. Hinderance to implementation : #'org-id-find does not report marker element for id containing search parameter.

Function in question

(defun org-transclusion-add-org-id (link plist)
  "Return a list for Org-ID LINK object and PLIST.
Return nil if not found."
  (when (string= "id" (org-element-property :type link))
    ;; when type is id, the value of path is the id
    (let* ((id (org-element-property :path link))
           (mkr (ignore-errors (org-id-find id t)))
           (payload '(:tc-type "org-id")))
      (if mkr
          (append payload (org-transclusion-content-org-marker mkr plist))
        (message
         "No transclusion done for this ID. Ensure it works at point %d, line %d"
         (point) (org-current-line))
        nil))))

point of failure : (mkr (ignore-errors (org-id-find id t)))

Possible interim patch

(defun patch/org-id-find--support-search (orig-fun id &optional markerp)
  "Enhance `org-id-find` to handle `id:UUID::*SEARCH` syntax.
ID may include a search string in the form `id:UUID::*SEARCH`.
When MARKERP is non-nil, return the marker position."
  (let* ((id-parts (split-string id "::" 'omit-nulls))
         (uuid (car id-parts))
         (search (cadr id-parts))
         (result (funcall orig-fun uuid markerp)))
    (if (and search result)
        (with-current-buffer (find-file-noselect (car (funcall orig-fun uuid)))
          (org-link-search search)
          (if markerp (point-marker)
	    (cons (buffer-file-name) (point))))
      result)))

(advice-add 'org-id-find :around #'patch/org-id-find--support-search)

Tested lightly.

Suggestion

Wait and watch upstream implementation. If org-id-find is patched to report marker positions for IDs containing search parameters - no change in org-transclusion is requisite.

akashpal-21 avatar Jan 07 '25 00:01 akashpal-21

@akashpal-21 , I am considering a more generic solution: 823982357fb8d33e4b3fcac5ba4234624204a3cd. More later :)

nobiot avatar Jan 07 '25 07:01 nobiot

For got to mention pr: #267

nobiot avatar Jan 07 '25 07:01 nobiot