girzel / ebdb

An EIEIO port of BBDB, Emacs' contact-management package
67 stars 12 forks source link

Add some org-roam integration #109

Closed swflint closed 6 months ago

swflint commented 1 year ago

I'd like to add some org-roam integration to EBDB.

The start will be a section for the org-roam-buffer, and ideally, a way to integrate notes.

girzel commented 1 year ago

Thanks for the proposal! I know almost nothing about org-roam, so this might take me a second.

Also, org-roam is an installable third-party package, right? I'm thinking about initialization -- the usual guidance is to not ask users to load features by requiring libraries, but instead by running initialization functions or setting customization options. I'm not clear on how this code would actually be used, though -- where would the user put this function? Also, it requires magit-section! There should be a require for magit-section at the top of the file, and that raises the further question of what happens for users who haven't installed one or both org-roam or magit-section. Or does org-roam already require magit section?

I suppose if it were autoloaded then it would be okay: users who don't want it simply wouldn't use it, and users who do want it would presumably already have the requirements in place and shouldn't be surprised to see an error if they don't.

But could you briefly describe how this would be used?

swflint commented 1 year ago

Thanks for the proposal! I know almost nothing about org-roam, so this might take me a second.

Also, org-roam is an installable third-party package, right? I'm thinking about initialization -- the usual guidance is to not ask users to load features by requiring libraries, but instead by running initialization functions or setting customization options. I'm not clear on how this code would actually be used, though -- where would the user put this function? Also, it requires magit-section! There should be a require for magit-section at the top of the file, and that raises the further question of what happens for users who haven't installed one or both org-roam or magit-section. Or does org-roam already require magit section?

I believe org-roam already requires magit-section, though I'd need to verify. There's a reason this is still a draft, as is. In addition to this, I'd like to see if there's a way to link to org-roam notes from within EBDB.

I suppose if it were autoloaded then it would be okay: users who don't want it simply wouldn't use it, and users who do want it would presumably already have the requirements in place and shouldn't be surprised to see an error if they don't.

Agreed.

But could you briefly describe how this would be used?

Sure! When you use org-roam you have the option of showing a "side buffer" when you show different roam nodes. Among things displayed in this side buffer are reference links and back-links to the current node. This adds a new thing that can be displayed, namely formatted EBDB entries when a node refers to an EBDB entry.

girzel commented 1 year ago

Aha! This is actually what the ebdb-mua.el functionality is for. It is misleadingly named for "mail user agent", but it isn't only for mail agents, it's meant for any software package that wants EBDB integration, which usually means finding/creating/displaying EBDB records from data provided by the package.

I would love to get this working with org-roam.el, mostly because this would be the first non-MUA "MUA" to get integration, but it will require some changes to the general infrastructure to get working. Right off the bat I know that mua integration wants to display relevant EBDB records in its own pop-up EBDB buffer, but that's not what we want here, we want to actually pass the record(s) to magit section.

The usual thing to is bind the ebdb-mua-keymap to a prefix in whatever mode map the user will be in. Then provide implementations of ebdb-mua-message-header that will find and return the strings used to search for or create EBDB records. But then we run into the problem of what to do with the returned records. I'll find some time to look at org-roam and magit-section.

girzel commented 1 year ago

I've looked into this a bit, and maybe there's no need to bring the whole ebdb-mua code into play. That's really more suited for situations where there may be multiple contacts present, and you may be wanting to update your EBDB database based on new incoming information.

Can you tell me what trigger mechanism is used to display these "side buffers"? Is there a hook (ie this is happening automatically), or a keybinding (ie the user has to request it)? And the bare-ref value you're pulling in your code, is that coming from the ID property of the node, or heading text, or might it come from somewhere else? You've only got this set up to pull a UUID, would it ever make sense to use the ref as a tags search, or a plain text search?

Thanks!

hokreb commented 6 months ago

@swflint: do you still work on the org-roam integration? I would also like to see a kind of integration of org roam with ebdb. Is the code you put into the pull request ready to running with org roam? If yes, how can I activate it, so it shows the addresses in the side window?

About which way was your idea to integrate notes with ebdb?

I am not sure, but I think, if one can integrate the linking to addresses as org-ids (by the uuid each address is assigned in the ebdb), this might be a direct way to integrate everything in org / org-roam universe. If one can specify by [[id:c8878aec-4c58-45656-9df3-ba70d124d50f]] directly an addres, org/org-roam would handle this like any other node/heading. If starting ebdb-2-org-id just add all the ids of the addresses/organisations as org-ids, this would be everything one needs. But I must admit, that I don't have a good elisp/org/org-roam knowledge, to judge, if this might be the way to go.

swflint commented 6 months ago

I haven't worked on this in a while, but yes, the code works (at least for me). You should just need to load the file ebdb-roam.el after ebdb & org-roam, then add ebdb-org-roam-section to your org-roam-sections. The section is also compatible with universal-sidecar through universal-sidecar-roam.

hokreb commented 6 months ago

@swflint: Thanks a lot for you explanations! I tried to add the code, but nothing is displayed in the side board org-roam when a link to an ebdb address is present in the org-roam node.

I tracked the single functions you use to get the links. You use the function org-roam-node-refs, this functions seems to return all the references found in the org roam file property :ROAM_REFS:, but no other links found in the the body of the org-roam node. Is this the intention of the code? When I try to add an ebdb address link in the :ROAM_REFS: property I get an error of org-roam (Invalid ref ... skipping). Do I miss there something?

So I am really not sure, if I have unterstand your intention correctly, I thought, this function would display all addresses of links to an ebdb entry which are present in the current org-roam node, so that's it is easy to get phone or emails or whatever else is saved in the database without any additional ebdb call, is this correct? At least I think, this could be a very useful use case.

swflint commented 6 months ago

That is correct. It only works for something found in :ROAM_REFS:. That being said, I suspect it would be reasonably easy to make it so that it can collect all ebdb links in the buffer/node.

hokreb commented 6 months ago

@swflint: Ok, so it was by intend to just use the :ROAM_REFS:.

I just tried to get also the information from the whole node, this is not so difficult. A first solution is working already. It has a small draw back, it will show all ebdb:uuid links of the file, it didn't differ between file node and heading node.

Here's the code for it, if it does interest you:

`(defun ebdb-org-roam-get-node-text-by-node (node) "Get the entire text content of an org-roam node file (not the single node only) by its node." (let ((file-path (org-roam-node-file node))) (if file-path (with-temp-buffer (insert-file-contents file-path) (buffer-substring-no-properties (point-min) (point-max))) nil)))

(defun ebdb-org-roam-extract-ebdb-uuid-links (text) "Extracts ebdb:uuid/ links from a given text string." (let ((start 0) links) (while (string-match "\[\[ebdb:uuid/\([a-zA-Z0-9-]+\)\]" text start) (push (match-string 1 text) links) (setq start (match-end 0))) (reverse links)))

(defun ebdb-org-roam-get-uuid-list (node) "get a list of ebdb uuids found in the whole file of org-roam node (without duplicates)." (when-let ((node-text (ebdb-org-roam-get-node-text-by-node node)) (uuid-list (ebdb-org-roam-extract-ebdb-uuid-links node-text))) (cl-remove-duplicates uuid-list :test #'string=)))

(defun ebdb-org-roam-section (node) "Show EBDB entry for current NODE." (when-let ((uuid-list (ebdb-org-roam-get-uuid-list node))) (magit-insert-section (org-roam-ebdb-section) (magit-insert-heading "Address Book Entry") (dolist (entry uuid-list) (insert (ebdb-fmt-record ebdb-default-multiline-formatter (ebdb-gethash entry 'uuid)))) (insert "\n\n")))) `

swflint commented 6 months ago

Hmm. It may make more sense to use org-elements to get the links as it's a bit cleaner (for instance, see this code), especially if you can figure out a way to get the bounds of a node within the file.

swflint commented 6 months ago

Try something along these lines:

(defun ebdb-roam--get-node-text (node)
  "Get text of NODE."
  (let ((file-path (org-roam-node-file node)))
    (if file-path
        (with-temp-buffer
          (insert-file-contents file-path)
          (buffer-substring-no-properties (org-roam-node-point node) (org-roam-node-marker node)))
      nil)))

(defun ebdb-roam--get-non-ref-links (node)
  "Get non-reference EBDB links for NODE."
  (cl-remove-duplicates (cl-remove-if (complement #'stringp)
                                      (org-element-map (with-temp-buffer
                                                         (ebdb-roam--get-node-text node)
                                                         (org-mode)
                                                         (org-element-parse-buffer))
                                          'link
                                        (lambda (link)
                                          (when-let ((link-type-p (string= "ebdb" (org-element-property :type link)))
                                                     (uuid-link (string-match-p (rx eol "uuid/") (org-element-property :path link))))
                                            (substring uuid-link 5)))))
                        :test #'string=))
swflint commented 6 months ago

Tbh, I'm not positive that org-roam-node-point and org-roam-node-marker are the way to get the start and end of the node, but I haven't taken a lot of time to do so.

swflint commented 6 months ago

Perhaps, more specifically, try this implementation and let me know how it works:

(defun ebdb-roam--get-node-text (node)
  "Get text of NODE."
  (let ((file-path (org-roam-node-file node)))
    (if file-path
        (with-temp-buffer
          (insert-file-contents file-path)
          (buffer-substring-no-properties (org-roam-node-point node) (org-roam-node-marker node)))
      nil)))

(defun ebdb-roam--get-non-ref-links (node link)
  "Get non-reference EBDB links for NODE."
  (let ((refs (cl-remove-duplicates
               (cl-remove-if (complement #'stringp)
                             (org-element-map (with-temp-buffer
                                                (ebdb-roam--get-node-text node)
                                                (org-mode)
                                                (org-element-parse-buffer))
                                 'link
                               (lambda (link)
                                 (when-let ((link-type-p (string= "ebdb" (org-element-property :type link)))
                                            (uuid-link (string-match-p (rx eol "uuid/") (org-element-property :path link))))
                                   (substring uuid-link 5)))))
               :test #'string=)))
    (if link
        (cons link refs)
      refs)))

(defun ebdb-roam-section (node)
  "Show EBDB entry for current NODE."
  (when-let ((bare-ref (car (org-roam-node-refs node)))
             (references (ebdb-roam--get-non-ref-links node
                                                       (when (string-match-p "^uuid/" bare-ref)
                                                         (substring bare-ref 5))))
             (entries (cl-remove-if #'null
                                    (mapcar (lambda (reference)
                                              (ebdb-gethash reference 'uuid))
                                            references))))
    (magit-insert-section (org-roam-ebdb-section)
      (magit-insert-heading "Address Book Entries")
      (dolist (entry entries)
        (insert (ebdb-fmt-record ebdb-default-multiline-formatter entry))
        (insert "\n\n")))))
swflint commented 6 months ago

Or perhaps try this:

(defun ebdb-roam--get-node-text (node)
  "Get text of NODE."
  (let ((file-path (org-roam-node-file node)))
    (if file-path
        (with-temp-buffer
          (insert-file-contents file-path)
          (buffer-substring-no-properties (org-roam-node-point node) (org-roam-node-marker node)))
      nil)))

(defun ebdb-roam--get-links (node)
  "Get non-reference EBDB links for NODE."
  (let ((bare-ref (car (org-roam-node-refs node)))
        (link (and (stringp bare-ref)
                   (string-match-p "^uuid/" bare-ref)
                   (substring bare-ref 5))))
    (cl-remove-duplicates
     (cl-remove-if (complement #'stringp)
                   (cons link
                         (org-element-map (with-temp-buffer
                                            (ebdb-roam--get-node-text node)
                                            (org-mode)
                                            (org-element-parse-buffer))
                             'link
                           (lambda (link)
                             (when-let ((link-type-p (string= "ebdb" (org-element-property :type link)))
                                        (uuid-link (string-match-p (rx eol "uuid/") (org-element-property :path link))))
                               (substring uuid-link 5))))))
     :test #'string=)))

(defun ebdb-roam-section (node)
  "Show EBDB entry for current NODE."
  (when-let ((references (ebdb-roam--get-links node))
             (entries (cl-remove-if #'null
                                    (mapcar (lambda (reference)
                                              (ebdb-gethash reference 'uuid))
                                            references))))
    (magit-insert-section (org-roam-ebdb-section)
      (magit-insert-heading "Address Book Entries")
      (dolist (entry entries)
        (insert (ebdb-fmt-record ebdb-default-multiline-formatter entry))
        (insert "\n\n")))))
girzel commented 6 months ago

I'm still into adding this! In the interest of getting it out and getting users, let's just do whatever works, and refine it later. If @hokreb is okay with the above code, let's just go with that?

(Briefly: cl-remove-if #'null is much more efficiently written as delq nil)

swflint commented 6 months ago

Wonderful. I haven't tested the above fully (I don't use non ROAM_REFS ebdb links), so if it does seem to work, I'll add it into the PR itself.

hokreb commented 6 months ago

Perfect, I will test it today

hokreb commented 6 months ago

So, out of the box, non of the three code block does run with valid results. As I am not really proficient with elisp, it will take me a little bit more time to find the source for errors, but I'd love to try to get a little bit more experience in elisp.

swflint commented 6 months ago

What errors are you seeing? I almost assuredly made some mistakes as I don't use the workflow you appear to. It may be helpful to add (debug (org-roam-node-point node) (org-roam-node-marker node)) in ebib-roam--get-node-text, because I suspect that's where an error lies.

I'll probably integrate the code as it stands, and we can iterate on the branch instead.

swflint commented 6 months ago

I was able to solve at least two errors, and I'll push them onto the branch shortly.

For a little bit of context: I used let when I should have used let* in ebdb-roam--get-links (let does bindings in parallel, thus bare-ref wasn't available for use in the expression generating the value for link), and the complement function isn't available in elisp, so I replaced it with cl-remove-if-not.

hokreb commented 6 months ago

Sorry for my long absent, I was heavily absorbed from my job. I test the last checkin, but it does not return the correct result.

I create a test org-roam file like this

:PROPERTIES:
:ID:       8ecdb1cb-1004-4c63-a52a-5402adb61844
:END:
#+title: ebdb-roam-test
#+filetags: 
#+startup: indent showall
* International Digital Security Forum Vienna

* People Meet

- [[ebdb:uuid/47a7a7be-1344-46dd-8075-40d6c355b0ed]]

* Test node 1
:PROPERTIES:
:ID:       113c4741-5e89-4169-bdea-4c5da1b29144
:END:

- [[ebdb:uuid/bf6f6dd0-3086-4405-85bf-6ec1f4e89418]]

** Test node 2

* Test node 3

not belonging to test node

and then tested the inner part of the ebdb-roam--get-links in a scratch buffer with the following results:

(setq node1 "8ecdb1cb-1004-4c63-a52a-5402adb61844")
"8ecdb1cb-1004-4c63-a52a-5402adb61844"

(setq node2 "113c4741-5e89-4169-bdea-4c5da1b29144")
"113c4741-5e89-4169-bdea-4c5da1b29144"

(defun ebdb-roam--get-node-elements (node)
  (with-current-buffer (find-buffer-visiting (org-roam-node-file node))
    (save-excursion
      (org-with-wide-buffer
       (goto-char (org-roam-node-point node))
       (org-element-at-point)))))
ebdb-roam--get-node-elements

(ebdb-roam--get-node-text (org-roam-node-from-id node1))
(property-drawer (:standard-properties [1 1 14 62 68 0 nil top-comment element t (90 . 1) nil ...]))

(ebdb-roam--get-node-text (org-roam-node-from-id node2))
(headline (:standard-properties [247 247 261 400 400 0 (:title) nil element t (90 . 256) 327 ...] :pre-blank 0 :raw-value [org-element-deferred org-element--headline-raw-value (2 13) nil] :title [org-element-deferred org-element-property-2 (:raw-value) nil] :level 1 :priority nil ...))

(defun ebdb-roam--get-links (node)
  (org-element-map (with-current-buffer (find-buffer-visiting (org-roam-node-file node))
                     (save-excursion
                       (org-with-wide-buffer
                        (goto-char (org-roam-node-point node))
                        (org-element-at-point))))
      'link
    (lambda (link)
      (when-let ((link-type-p (string= "ebdb" (org-element-property :type link)))
                 (uuid-link (string-match-p (rx eol "uuid/") (org-element-property :path link))))
        (substring uuid-link 5)))))
ebdb-roam--get-links

(ebdb-roam--get-links (org-roam-node-from-id node1))
nil

(ebdb-roam--get-links (org-roam-node-from-id node2))
nil

It seems, that the inner part of the org-element-map gets some structure (I don't really know anything about this), but that the lambda (link) does not find a link in there.

swflint commented 6 months ago

Interesting. Hmm... I wonder if my element-at-point approach is faulty. It may instead be necessary to get the element-at-point & walk up the tree (using org-element-parent) until a section is found, then traverse that.

swflint commented 6 months ago

I've pushed some code to do just that, I think. It probably needs some more work, however.

hokreb commented 6 months ago

it doesn't work either. But just a question, would be an approach easier, where we just find each ebdb link in the whole org-roam-file first, add the point position of this link, and than go down the org header hierachie to check, if the way down is a PROPERTY ID is found?

I think the typical org-roam-file is more or less some kilobyte big, some maybe hundred kilobyte, so the search in the whole text will be fast enough.

I think this approach prevents also some interesting problem of org-roam. If you have an org-roam file with the typical file property id set, and than you have several org headers in the first hierachie level. If the first header has no property id, this header belongs to the file properties id. If the second header has a property id, this header creates a new node. If a third header follows, which has no property id, this header belongs to the file property id (at least, if I look at existing backlinks, this is how the nodes are named than.

Here is the code which implements the above strategy, it is working correct on my test sample and only shows the address book entries for the corresponding org roam nodes. I am sure, this can be improved (like moving the duplication stuff up to the ebdb-roam--get-links), but this is at least a working version


(defun ebdb-roam--get-node-id (point)
  "Return the ID property from the Org mode header at POINT, or from its parent headers.
   If no ID is found up to the top level, check file properties for an ID."
  (save-excursion  ; Preserve the current position of the point
    (goto-char point)  ; Move the point to the specified position
    (let ((id nil))
      ;; Loop to check each header level
      (while (and (not id) (not (eq (org-current-level) 1)))
        (setq id (org-entry-get (point) "ID"))
        (unless id  ; If ID not found, move up to the parent header
          (org-up-heading-safe)))
      ;; If no ID found in headers, check file properties
      (or id (org-entry-get nil "ID" t)))))  ; 't' to search in file properties

(defun ebdb-roam--get-links (org-roam-node)
  "Extract positions and text of EBDB links from an Org-roam node."
  (let ((file-path (org-roam-node-file org-roam-node))
        link-data)
    (when (and file-path (find-buffer-visiting file-path))
      (with-current-buffer (find-buffer-visiting file-path)
        (save-excursion
          (goto-char (point-min))
          (while (re-search-forward "\\[\\[ebdb:uuid/\\([^]]+\\)\\]" nil t)
            (let* ((position (point))
                  (link-text (match-string-no-properties 1))
                  (roam-id (ebdb-roam--get-node-id position))
                  (node-id (org-roam-node-id org-roam-node)))
              (if (string= roam-id node-id) (push link-text link-data)))))))
    link-data))

(defun ebdb-roam--get-uuid-list (node)
  "get a list of ebdb uuids found in the whole file of org-roam node (without duplicates)."
  (when-let ((uuid-list (ebdb-roam--get-links node)))
    (cl-remove-duplicates uuid-list :test #'string=)))

(defun ebdb-roam-section (node)
  "Show EBDB entry for current NODE."
  (when-let ((uuid-list (ebdb-roam--get-uuid-list node)))
    (magit-insert-section (org-roam-ebdb-section)
      (magit-insert-heading "Address Book Entry")
      (dolist (entry uuid-list)
        (insert (ebdb-fmt-record ebdb-default-multiline-formatter (ebdb-gethash entry 'uuid))))
      (insert "\n\n"))))
swflint commented 6 months ago

it doesn't work either. But just a question, would be an approach easier, where we just find each ebdb link in the whole org-roam-file first, add the point position of this link, and than go down the org header hierachie to check, if the way down is a PROPERTY ID is found?

That's somewhat unsurprising, I barely tested the code. Are you getting any sort of error? If so, what?

I think the typical org-roam-file is more or less some kilobyte big, some maybe hundred kilobyte, so the search in the whole text will be fast enough.

I agree on the size. That being said, I don't like the idea of using text search, mostly because if we decide we want to try to support other EBDB link types, it becomes significantly harder, as we'd have to modify the regexps a lot more. Using the org-elements API will make that a lot easier.

I think this approach prevents also some interesting problem of org-roam. If you have an org-roam file with the typical file property id set, and than you have several org headers in the first hierachie level. If the first header has no property id, this header belongs to the file properties id. If the second header has a property id, this header creates a new node. If a third header follows, which has no property id, this header belongs to the file property id (at least, if I look at existing backlinks, this is how the nodes are named than.

The text-based approach? Maybe. However I think getting the org-element containing the node means we don't have to try to manually detect the end-of-node point (more or less).

Here is the code which implements the above strategy, it is working correct on my test sample and only shows the address book entries for the corresponding org roam nodes. I am sure, this can be improved (like moving the duplication stuff up to the ebdb-roam--get-links), but this is at least a working version


(defun ebdb-roam--get-node-id (point)
  "Return the ID property from the Org mode header at POINT, or from its parent headers.
   If no ID is found up to the top level, check file properties for an ID."
  (save-excursion  ; Preserve the current position of the point
    (goto-char point)  ; Move the point to the specified position
    (let ((id nil))
      ;; Loop to check each header level
      (while (and (not id) (not (eq (org-current-level) 1)))
        (setq id (org-entry-get (point) "ID"))
        (unless id  ; If ID not found, move up to the parent header
          (org-up-heading-safe)))
      ;; If no ID found in headers, check file properties
      (or id (org-entry-get nil "ID" t)))))  ; 't' to search in file properties

(defun ebdb-roam--get-links (org-roam-node)
  "Extract positions and text of EBDB links from an Org-roam node."
  (let ((file-path (org-roam-node-file org-roam-node))
        link-data)
    (when (and file-path (find-buffer-visiting file-path))
      (with-current-buffer (find-buffer-visiting file-path)
        (save-excursion
          (goto-char (point-min))
          (while (re-search-forward "\\[\\[ebdb:uuid/\\([^]]+\\)\\]" nil t)
            (let* ((position (point))
                  (link-text (match-string-no-properties 1))
                  (roam-id (ebdb-roam--get-node-id position))
                  (node-id (org-roam-node-id org-roam-node)))
              (if (string= roam-id node-id) (push link-text link-data)))))))
    link-data))

(defun ebdb-roam--get-uuid-list (node)
  "get a list of ebdb uuids found in the whole file of org-roam node (without duplicates)."
  (when-let ((uuid-list (ebdb-roam--get-links node)))
    (cl-remove-duplicates uuid-list :test #'string=)))

(defun ebdb-roam-section (node)
  "Show EBDB entry for current NODE."
  (when-let ((uuid-list (ebdb-roam--get-uuid-list node)))
    (magit-insert-section (org-roam-ebdb-section)
      (magit-insert-heading "Address Book Entry")
      (dolist (entry uuid-list)
        (insert (ebdb-fmt-record ebdb-default-multiline-formatter (ebdb-gethash entry 'uuid))))
      (insert "\n\n"))))

I'll have to take a closer look at the code for this to better understand it.

hokreb commented 6 months ago

You are totally correct, it would be better to use the underlaying structure of org mode. But I must admit, I do not know how these structure of org-files is build, that's makes it rather complicate for me :-). Therefore sometimes I just do a working prototype, which solves the problem, but which isn't the most elegant one. And than try, if time allows, to find the elegant solution in a second step.

But I have another question, I would like to build a small contact relation management system by using ebdb and org-roam. The first step was to see the address of ebdb links in org-roam in the side window. If have the idea, that I just can link org-roam daily notes to persons I meet and talked to in business related topic's and then can integrate these connections in the org-roam-ui-mode. I found somewhere, that it is possibily to link citations of paper/literature to org-roam nodes and these citations will show up in the org-roam-ui-mode. I tried to replicate the setup for showing me the citations in org-roam-ui-mode, but failed to replicate it. Has you been able to use citation in such a way? Or do you now, if this could easily be extended to ebdb:uuid links?

swflint commented 6 months ago

You are totally correct, it would be better to use the underlaying structure of org mode. But I must admit, I do not know how these structure of org-files is build, that's makes it rather complicate for me :-). Therefore sometimes I just do a working prototype, which solves the problem, but which isn't the most elegant one. And than try, if time allows, to find the elegant solution in a second step.

I get that! The docs are pretty good, which helps a lot. Tbh, I think the big thing is just figuring out how to make sure we get the right element. The code may just be using the wrong code to get the element at point (I'm not super familiar with the API either.

But I have another question, I would like to build a small contact relation management system by using ebdb and org-roam. The first step was to see the address of ebdb links in org-roam in the side window. If have the idea, that I just can link org-roam daily notes to persons I meet and talked to in business related topic's and then can integrate these connections in the org-roam-ui-mode. I found somewhere, that it is possibily to link citations of paper/literature to org-roam nodes and these citations will show up in the org-roam-ui-mode. I tried to replicate the setup for showing me the citations in org-roam-ui-mode, but failed to replicate it. Has you been able to use citation in such a way? Or do you now, if this could easily be extended to ebdb:uuid links?

That, I'm not familiar with. I suspect you'd just have to hook into some things in org-roam-ui, but I don't use it or program for it, so I don't know where to start.

hokreb commented 6 months ago

So, I looked a little bit in the org-roam database, and I think there I found one elegant solution to get the ebdb links, as the database contains these link types.

Now I use the following solution to find all ebdb links (as you @swflint mentioned) , you can even specifiy other ebdb link types if needed with an optional argument. I also found a way to directly avoid adding duplicates to the returned list.

(defun ebdb-roam--get-links (uuid &optional specified-type)
  "Query org-roam database for 'ebdb' links from a specific UUID and of a specified type."
  (let ((query-result (org-roam-db-query [:select [dest]
                                          :from links
                                          :where (and (= type "ebdb")
                                                      (= source $s1))]
                                         uuid))
        (desired-type (or specified-type "uuid"))
        address-list)

    (dolist (row query-result)
      (let* ((dest (car row))
             (split-dest (split-string dest "/"))
             (dest-type (car split-dest))
             (dest-address (cadr split-dest)))        
        (when (string-equal dest-type desired-type)          
          (pushnew dest-address address-list :test #'equal))))
    address-list))

(defun ebdb-roam-section (node)
  "Show EBDB entry for current NODE."
  (when-let ((uuid-list (ebdb-roam--get-links (org-roam-node-id node))))
    (magit-insert-section (org-roam-ebdb-section)
      (magit-insert-heading "Address Book Entry")
      (dolist (entry uuid-list)
        (insert (ebdb-fmt-record ebdb-default-multiline-formatter (ebdb-gethash entry 'uuid))))
      (insert "\n\n"))))

This code I tested within my org-roam installation and my ebdb links contained within, and it works out of the box. So what do you think @swflint? It would be nice if you could do a code review to remove any of my elisp beginner errors from the code, so that it might be incorporated from @girzel, or is this approach problematic?

swflint commented 6 months ago

I've taken a look. Since it appears that your approach is able to get the links other than the ROAM_REFS, it seems like it works overall. I've gone ahead and incorporated support for the ROAM_REFS as well, and cleaned up the code a bit.

Try this:

(defun ebdb-roam--get-links (node &optional specified-type)
  "Query org-roam database for 'ebdb' links from a specific UUID and of a specified type."
  (let* ((node (if (org-roam-node-p node)
                   node
                 (org-roam-node-from-id node)))
         (query-result (org-roam-db-query [:select [dest]
                                                   :from links
                                                   :where (and (= type "ebdb")
                                                               (= source $s1))]
                                          (org-roam-node-id node)))
         (bare-ref (car (org-roam-node-refs node)))
         (link (and (stringp bare-ref)
                    (string-match-p "^uuid/" bare-ref)
                    bare-ref))
         (desired-type (or specified-type "uuid")))
    (cl-remove-duplicates
     (delq nil
           (mapcar (lambda (row)
                     (unless (null row)
                       (let* ((dest (car row))
                              (split-dest (split-string dest "/"))
                              (dest-type (car split-dest))
                              (dest-address (cadr split-dest)))
                         (and (string-equal dest-type desired-type) dest-address))))
                   (cons (list bare-ref)
                         query-result)))
     :test #'string=)))

(defun ebdb-roam-section (node)
  "Show EBDB entries for current NODE."
  (when-let ((uuid-list (ebdb-roam--get-links node)))
    (magit-insert-section (org-roam-ebdb-section)
      (magit-insert-heading "Address Book Entries")
      (dolist (uuid uuid-list)
        (when-let ((entry (ebdb-gethash uuid 'uuid)))
          (insert (ebdb-fmt-record ebdb-default-multiline-formatter entry))))
      (insert "\n"))))
swflint commented 6 months ago

Also, you can make sure that your code is syntax highlighted by including the name of the language on the same line as the opening triple-backticks. For Elisp, use elisp

hokreb commented 6 months ago

Shouldn't the (string-match-p "uuid/" bare-ref) use the desired-type to keep it similar?

swflint commented 6 months ago

Yes, thank you for catching that.

hokreb commented 6 months ago

I just add the the line

#+ROAM_REFS: [[ebdb:uuid/8b02b79f-9e48-4ec9-bc17-ccb01220cb27]]

to my org roam file, to test also ROAM_REFS entries. This link is already added by the database query, you don't need to add bare-ref handling.

hokreb commented 6 months ago

That would mean, this code should be the final one

(defun ebdb-roam--get-links (node &optional specified-type)
  "Query org-roam database for 'ebdb' links from a specific UUID and of a specified type."
  (let* ((node (if (org-roam-node-p node)
                   node
                 (org-roam-node-from-id node)))
         (query-result (org-roam-db-query [:select [dest]
                                                   :from links
                                                   :where (and (= type "ebdb")
                                                               (= source $s1))]
                                          (org-roam-node-id node)))
         (desired-type (or specified-type "uuid")))
    (cl-remove-duplicates
     (delq nil
           (mapcar (lambda (row)
                     (unless (null row)
                       (let* ((dest (car row))
                              (split-dest (split-string dest "/"))
                              (dest-type (car split-dest))
                              (dest-address (cadr split-dest)))
                         (and (string-equal dest-type desired-type) dest-address))))
                   query-result))
     :test #'string=)))

(defun ebdb-roam-section (node)
  "Show EBDB entries for current NODE."
  (when-let ((uuid-list (ebdb-roam--get-links node)))
    (magit-insert-section (org-roam-ebdb-section)
      (magit-insert-heading "Address Book Entries")
      (dolist (uuid uuid-list)
        (when-let ((entry (ebdb-gethash uuid 'uuid)))
          (insert (ebdb-fmt-record ebdb-default-multiline-formatter entry))))
      (insert "\n"))))
swflint commented 6 months ago

Except ROAM_REFS is often a property in the property drawer, and IIRC, is supposed to be a bare link (i.e., [[bare-link]] without the brackets), so special handling is necessary.

swflint commented 6 months ago

See Section 8.4 of the Org Roam manual.

hokreb commented 6 months ago

Yes, but also without the brackets, the link is found correctly. I think, this will be handled by org-roam itselfs, when the database is updated That would mean a link like this: '#+ROAM_REFS: ebdb:uuid/8b02b79f-9e48-4ec9-bc17-ccb01220cb27'

swflint commented 6 months ago

Interesting, I don't see it happening when it is in a property drawer, however.

swflint commented 6 months ago

Yes, but also without the brackets, the link is found correctly. I think, this will be handled by org-roam itselfs, when the database is updated That would mean a link like this: '#+ROAM_REFS: ebdb:uuid/8b02b79f-9e48-4ec9-bc17-ccb01220cb27'

Again, ROAM_REFS canonically are stored in the property drawer (see also org-roam-ref-add, and org-roam-property-add, which uses org-set-property).

Additionally, per the v1 migration FAQ, ROAM_REFS is a property.

hokreb commented 6 months ago

Yes, you are right, I didn't thought on the property drawer. This could be handled by the org-roam database refs table:

(defun ebdb-roam--get-links (node &optional specified-type)
  "Query org-roam database for 'ebdb' links from a specific UUID and of a specified type."
  (let* ((node (if (org-roam-node-p node)
                   node
                 (org-roam-node-from-id node)))
         (uuid (org-roam-node-id node))
         (query-result-links (org-roam-db-query [:select [dest]
                                                         :from links
                                                         :where (and (= type "ebdb")
                                                                     (= source $s1))]
                                                uuid))
         (query-result-refs (org-roam-db-query [:select [ref]
                                                        :from refs
                                                        :where (and (= type "ebdb")
                                                                    (= node_id $s1))]
                                               uuid))
         (query-results (append query-result-refs query-result-links))
         (desired-type (or specified-type "uuid")))
    (cl-remove-duplicates
     (delq nil
           (mapcar (lambda (row)
                     (unless (null row)
                       (let* ((dest (car row))
                              (split-dest (split-string dest "/"))
                              (dest-type (car split-dest))
                              (dest-address (cadr split-dest)))
                         (and (string-equal dest-type desired-type) dest-address))))
                   query-results))
     :test #'string=)))

(defun ebdb-roam-section (node)
  "Show EBDB entries for current NODE."
  (when-let ((uuid-list (ebdb-roam--get-links node)))
    (magit-insert-section (org-roam-ebdb-section)
      (magit-insert-heading "Address Book Entries")
      (dolist (uuid uuid-list)
        (when-let ((entry (ebdb-gethash uuid 'uuid)))
          (insert (ebdb-fmt-record ebdb-default-multiline-formatter entry))))
      (insert "\n"))))
swflint commented 6 months ago

That looks great! I'll move it onto the branch soon.

hokreb commented 6 months ago

One question, is the (delq nil) really needed? Both queries return not a (list nil) but only a nil value in case there is nothing found, so there seems no possibility, that a list with a nil value can be created?

girzel commented 6 months ago

The (and (string-equal dest-type desired-type) dest-address) could in theory return a nil. I don't know if that could actually happen in practice, though.

swflint commented 6 months ago

I think it could easily, if there's a non-uuid link.

swflint commented 6 months ago

@hokreb How should I list you as an author?

swflint commented 6 months ago

I've switched to the new implementation, cleaned up the doc string, and made the section a bit more customizable.

hokreb commented 6 months ago

@hokreb How should I list you as an author?

Is it possible to just just my name here: hokreb?

swflint commented 6 months ago

I can do that!

girzel commented 6 months ago

Let me know what the next steps are! I've never done a PR review/acceptance on Github, but here we are, so...

swflint commented 6 months ago

It's now ready for review. You can make comments in-line on the "files changed" tab, and once you are satisfied with the code, you can either merge on the command line (or using magit, forge, etc.), or with the buttons at the bottom of the page.