axbweiss commented 3 months ago

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.

nobiot commented 3 months ago

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

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

axbweiss commented 3 months ago

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)

nobiot commented 3 months ago

@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?

akashpal-21 commented 3 months ago

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 commented 3 months ago

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.

axbweiss commented 3 months ago

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.

nobiot commented 3 months ago

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

akashpal-21 commented 3 months ago

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.

axbweiss commented 3 months ago

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 commented 3 months ago

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!

nobiot commented 3 months ago

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

axbweiss commented 3 months ago

@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)

akashpal-21 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

nobiot / org-transclusion

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

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.