Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question on org-roam-node-insert and org-roam-node-find #2431

Open
deb75 opened this issue Apr 28, 2024 · 4 comments
Open

Question on org-roam-node-insert and org-roam-node-find #2431

deb75 opened this issue Apr 28, 2024 · 4 comments

Comments

@deb75
Copy link

deb75 commented Apr 28, 2024

Brief Abstract

I want org-roam-node-insert and org-roam-node-find to insert and return the chosen node with its parent as :

parent node title/chosen node title

Proposed Implementation (if any)

Here is an implementation of this for org-roam-node-find :

(with-eval-after-load 'org-roam
  (cl-defmethod org-roam-node-mytitle ((node org-roam-node))
    "Return the TITLE of NODE."
    (let ((level (org-roam-node-level node))
          (title (org-roam-node-title node)))
      (cond
       ((= level 0) title)
       ((= level 1) (concat (org-roam-node-file-title node) "/" title))
       (t (save-window-excursion
            (save-excursion
              (org-roam-node-open node nil t)
              (org-roam-up-heading-or-point-min)
              (concat (org-roam-node-title (org-roam-node-at-point)) "/" title))))))))

(setq org-roam-node-display-template
      (concat "${type:20} ${mytitle:60} " (propertize "${tags:30}" 'face 'org-tag)))

This implementation defines a custom function to retrieve the title of the node. If the node level is the root (0),
the title is returned as is, if level is 1, this means it is a first header of a file and the parent node is the file title
itself, otherwise I have to parse the file back up to the upper level node.

This implementation works well :
cap

Problem

My current issue is that with an increasing number of nodes (about a thousand), this
implementation becomes slow, not because of the number of nodes itself but because
all files are opened and there is some org processing each time :

Position saved to mark ring, go back with ‘M-x org-mark-ring-goto’. [334 times]

But I do not need all this org processing to just retrieve the parent node.

Request

  • Do you know how I can improve this implementation to make it faster ?
  • I look for a similar implementation for org-roam-node-insert so that inserting a link in a file
    would result in parent node title/linked node title, but it seems harder to implement at first sight,
    how could I proceed ?

To my knowledge : [ X] No similar feature requests

@akashpal-21
Copy link

akashpal-21 commented May 20, 2024

I don't grasp the problem here - org-roam should not open any files for processing - it queries a db; maybe the files are being opened by org-agenda? The files are opened because you have asked to open them for the 't' type ( all other case )

You want the find to be fast?
Also want to insert with a custom description?

To make the find fast we can cache the formatting by introducing a cache-mechanism using hash-tables,
Changing the insert description could also be done easily.
Please post in the discourse if youd need help with any of these, or search them.

(defvar cache/org-roam-node-read--to-candidate (make-hash-table :test 'equal))
(defun advice/org-roam-node-read--to-candidate (fn &rest args)
  (or (gethash args cache/org-roam-node-read--to-candidate)
      (let ((result (apply fn args)))
        (puthash args result cache/org-roam-node-read--to-candidate)
        result)))
(advice-add 'org-roam-node-read--to-candidate :around #'advice/org-roam-node-read--to-candidate)

Recently I came across this cache - it would work well for your case, for larger node sizes we can cache more things.

EDIT
The files are opened because you have specified for the t - type to do this. This is part of your configuration.

@deb75
Copy link
Author

deb75 commented May 21, 2024

I came up with this :

(with-eval-after-load 'org-roam
  (cl-defmethod org-roam-node-mytitle ((node org-roam-node))
    "Return the parent node of NODE."
    (let ((level (org-roam-node-level node))
          (title (org-roam-node-title node)))
      (cond
       ((= level 0) title)
       ((= level 1) (concat (org-roam-node-file-title node) "/" title))
       (t (let* ((file (org-roam-node-file node))
                 (marker (org-roam-node-marker node)))
            (with-temp-buffer
              (insert-file-contents file)
              (goto-char marker)
              (org-up-heading-or-point-min)
              (concat (org-roam-node-title (org-roam-node-from-id (org-entry-get (point) "ID" t))) "/" title))))))))

which is faster.

I am still interested in customizing node insertion (C-c n i).

@akashpal-21
Copy link

This is a very inefficient way - there are a lot of redundant things being done.
The final candidate format preparation is the most expensive process in getting the final list of candidates.

If you create such inefficiencies - they shall scale up and make your day to day operation a nightmare.

Q) Why are you creating a temp buffer?
(org-roam-node-title (org-roam-node-from-id (org-entry-get (point) "ID" t))) "/

what is the meaning of this? You are getting the node struct as an argument to this function - you are going around the world to get it again.

Please consider not creating such inefficiencies -- unless you cache - this list is being prepared over and over when you call the read function which gives you the interface to choose a node.

Can you please come to the discourse -- it is not possible to talk extensively here -- your case is not a issue - but if you require help, youd get better help there.

@akashpal-21
Copy link

(cl-defmethod org-roam-node-hierarchy ((node org-roam-node))
  (let ((level (org-roam-node-level node)))
    (concat
     (when (> level 0) (concat (propertize (org-roam-node-file-title node) 'face 'italic) " / "))
     (when (> level 1) (concat (string-join (org-roam-node-olp node) " > ") " > "))
    (propertize  (org-roam-node-title node) 'face 'bold))))

This is what I use - do not define a t type -- all you need is two scenarios. I use Bold and Italic to differentiate - but you can as easily remove them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants