jgru/consult-org-roam

Rough idea: add source for consult-buffer

apc opened this issue · 7 comments

apc commented

This is related to this issue over at the consult repo.

Feature request

I'm looking for a way of getting consult-buffer to treat org-roam buffers in a way that's more useful (to me at least). Basically, I'd want org-roam buffers to be listed among the candidates generated by consult-buffers using the title rather than the buffer name. The main reason is that I name all my org-roam files using just a timestamp, and that isn't especially informative when trying to find a particular buffer.

A start

As a starting point, I came up with something that sort of works. The main idea is simple and I think on the right track, but the implementation is not quite there yet.

The main idea

The proposal is to define a new source to add to consult-buffer-sources which will consist of those buffers listed in (org-roam-buffer-list). This source will list items by passing to consult--buffer-query' a conversion function that will extract the title of the buffer. The key is to make sure that the state function knows to find the corresponding buffer given a title.

A few things are needed:

  1. A 'translation' mechanism that allows us to get buffers from titles and vice versa.
  2. Two functions that will mimic consult--buffer-preview and consult--buffer-action but which will work on titles of org-roam buffers rather than buffer names.
  3. A way of annotating the list of org-roam titles.

Where I am at

Thanks to @minad, I have what is at least a good enough solution to 2.

As for 3, I haven't figured out how to do it in a consult only way. But using marginalia it's fairly straightforward.

The sticking point is 1. I have a solution that sort of works but it's probably not good enough. For one thing, it presupposes no too org-roam buffers have the same title, which is not a reasonable presupposition to make.

Annotated proposal

Here at last is what I got. I'm sure it can be improved upon in many ways, so take this just as a starting point.

First, we need a translation mechanism. The buffer-to-title translation is easy. The title-to-buffer translation needs some work.

(defun org-roam-buffer-get-title (buffer)
  "Get title for BUFFER when it corresponds to an org-roam buffer"
  (when (org-roam-buffer-p buffer)
        (with-current-buffer buffer (org-roam-db--file-title))))

(defun org-roam-buffer-add-title (buffer)
  "Build a cons consisting of the BUFFER title and the BUFFER name"
  (cons (org-roam-buffer-get-title buffer) buffer))

(defun org-roam-buffer-update-open-buffer-list ()
  "Generate an alist of the form `(TITLE . BUF)’, where TITLE is
the title of an open org-roam BUFfer"
  (setq org-roam-buffer-open-buffer-list (mapcar #'org-roam-buffer-add-title (org-roam-buffer-list))))

(defun org-roam-buffer-with-title (title)
  "Find buffer name with TITLE from among the list of open org-roam
buffers"
  (org-roam-buffer-update-open-buffer-list)
  (cdr (assoc title org-roam-buffer-open-buffer-list)))

I could not find a sort of 'inverse' of org-roam-db--tile-title, but if there is one that may be a better bet than what I did to define org-roam-buffer-with-title.

We then need to define the analogue of consult--buffer-state (I think @minad had something even more efficient in mind, but I couldn't get his suggestion to work somehow):

(defun org-roam-buffer-state nil
  (let
      ((preview
        (consult--buffer-preview)))
    (lambda
      (action cand)
      (funcall preview action (org-roam-buffer-with-title cand))
      (when
          (and cand
               (eq action 'return))
        (consult--buffer-action (org-roam-buffer-with-title cand))))))

We can now define a new source for consult-buffer. Note that I'm using a custom category, which will be useful later when adding an annotator function for marginalia:

(autoload 'org-roam-buffer-list "org-roam")

(defvar org-roam-buffer-source
  `(:name     "Org-roam"
              :hidden   nil
              :narrow   ?r
              :category org-roam-buffer
              :state    ,#'org-roam-buffer-state
              :items    ,(lambda () (consult--buffer-query :sort 'visibility :as #'org-roam-buffer-get-title :filter nil :predicate (lambda (buf) (org-roam-buffer-p buf))))))

(add-to-list 'consult-buffer-sources 'org-roam-buffer-source 'append)

Finally, an annotator to use with marginalia:

(defun marginalia-annotate-org-roam-buffer (cand)
  "Annotate org-roam buffer titled CAND with modification status, file name and major mode."
(when (assoc cand org-roam-buffer-open-buffer-list)
  (marginalia-annotate-buffer (org-roam-buffer-with-title cand)))

(add-to-list 'marginalia-annotator-registry
             '(org-roam-buffer marginalia-annotate-org-roam-buffer builtin none))

If we put all of this together, we will still see the org-roam buffers included in consult--source-buffer:

image

In my case, I can filter them out using consult-buffer-filter, since all and only my org-roam files match a specific regexp. A more sophisticated approach would perhaps involve modifying the value of the :items key in consult--source-buffer so as to exclude org-roam-buffers. I haven't tested this, but presumably:

(consult-customize consult--source-buffer :items (lambda () (consult--buffer-query :sort 'visibility :as #'buffer-name :predicate (lambda (buf) (not (org-roam-buffer-p buf))))))

image

None of this will work without enabling lexical binding. I learned it the hard way.

jgru commented

Hi @apc,

thank you for drafting this idea. I really like and would like to incorporate such a capability into consult-org-roam.
I already had a look at your draft and made some modifications:

1. Addressing the "duplicate title issue":
I added the buffer backing file's hash as stored in the org-roam-DB to the title. This ensures that duplicate titles are handled correctly.

2. Addressing annotation:
I added an annotation function using consult's very own capabilities to display the name of the originating file.

The resulting code mainly building up on you initial effort is then as follows:

;;; consult-org-roam-buffer.el --- Consult-buffer integration for org-roam  -*- lexical-binding: t; -*-

(defun consult-org-roam-buffer--state ()
  (let ((preview (consult--buffer-preview)))
    (lambda
      (action cand)
      (funcall preview action
        (consult-org-roam-buffer--with-title cand))
      (when
        (and cand
          (eq action 'return))
        (consult--buffer-action
          (consult-org-roam-buffer--with-title cand))))))

(defun consult-org-roam-buffer--get-title (buffer)
  "Select from list of all notes that link to the current note."
  (if (org-roam-buffer-p buffer)
    (let* ((title
             (with-current-buffer buffer
               (org-roam-db--file-title)))
            (filename (buffer-file-name buffer))
            (fhash (org-roam-db--file-hash filename)))
      (progn (concat title " [" (substring fhash 0 7) "]")))))

(defun consult-org-roam-buffer--add-title (buffer)
  "Build a cons consisting of the BUFFER title and the BUFFER name"
  (cons (consult-org-roam-buffer--get-title buffer) buffer))

(defun consult-org-roam-buffer--update-open-buffer-list ()
  "Generate an alist of the form `(TITLE . BUF)’, where TITLE is the title of an open org-roam buffer"
  (setq org-roam-buffer-open-buffer-list
    (mapcar #'consult-org-roam-buffer--add-title
      (org-roam-buffer-list))))

(defun consult-org-roam-buffer--with-title (title)
  "Find buffer name with TITLE from among the list of open org-roam buffers"
  (consult-org-roam-buffer--update-open-buffer-list)
  (cdr (assoc title org-roam-buffer-open-buffer-list)))

(autoload 'org-roam-buffer-list "org-roam")
(add-to-list 'consult-buffer-sources 'org-roam-buffer-source 'append)

(defun consult-org-roam-buffer--get-roam-bufs ()
  "Return list of currently open org-roam buffers"
  (consult--buffer-query
    :sort 'visibility
    :as #'consult-org-roam-buffer--get-title
    :filter t
    :predicate (lambda (buf) (org-roam-buffer-p buf))))

(defvar org-roam-buffer-source
  `(:name     "Org-roam"
     :hidden   nil
     :narrow   ?r
     :category org-roam-buffer
     :annotate ,(lambda (cand)
                  (f-filename
                    (buffer-file-name
                      (consult-org-roam-buffer--with-title cand))))
     :state    ,#'consult-org-roam-buffer--state
     :items    ,#'consult-org-roam-buffer--get-roam-bufs))

(add-to-list 'consult-buffer-sources 'org-roam-buffer-source 'append)

;; Customize consult--source-buffer to show org-roam buffers only in
;; their dedicated section
(consult-customize
  consult--source-buffer
  :items (lambda ()
           (consult--buffer-query
             :sort 'visibility
             :as #'buffer-name
             :predicate (lambda (buf) (not (org-roam-buffer-p buf))))))

;;; consult-org-roam-buffer.el ends here

What do you think about these changes? Do we need to change anything else here?

Best regards,
jgru

apc commented

Great! I like this. Good to know there is some way of dealing with the title issue.

Quick question: what's the rationale for adding the hash of the file to the annotation? Not that I mind either way, just wondering.

apc commented

Oh, and one more question (maybe @minad can chime in here): is there some clean way of making this source be listed right after consult--source-buffer? As is, org-roam buffers end up buried at the bottom, and it makes more sense for them to be listed right after 'regular' buffers.

I tried manually setting the value of consult-buffer-sources in the right order and also hiding both consult--source-recent-files and consult--source-bookmarks and they work fine. But I do not know if there is some bit of elisp magic that will allow me to add an element to consult-buffer-sources right after some specific element (in this case, right after consult--source-buffer).

jgru commented

Great! I like this. Good to know there is some way of dealing with the title issue.

Glad to hear!

Quick question: what's the rationale for adding the hash of the file to the annotation? Not that I mind either way, just wondering.

The rationale is that the hash is unique and by using Some title [abcdef] as a key to find the buffer, we can be sufficiently certain that can deal with such duplicate titles since it results in an output like the following:

-----Org-roam-----
A test [5289ab7]  20221006154821-a_test.org
A test [ecf66d8]  20221006154803-a_test.org

(We could also use the datetime-prefix here, but since a hash is recorded in the DB I chose that.)

Oh, and one more question (maybe @minad can chime in here): is there some clean way of making this source be listed right after consult--source-buffer? As is, org-roam buffers end up buried at the bottom, and it makes more sense for them to be listed right after 'regular' buffers.
But I do not know if there is some bit of elisp magic that will allow me to add an element to consult-buffer-sources right after some specific element (in this case, right after consult--source-buffer).

I would propose to do it like so:

;; Find index to put
(setq idx
  (cl-position 'consult--source-buffer consult-buffer-sources :test 'equal))
;; Store the elements coming after the insertion point 
(setq tail (nthcdr idx consult-buffer-sources))
;; Insert element and set its cdr
(setcdr
  (nthcdr (1- idx) consult-buffer-sources)
  (append (list 'org-roam-buffer-source) tail))

Better solutions welcome.

So, do you think this new capability should be pushed as consult-org-roam-buffer.el?

Best regards,
jgru

apc commented

Great! I like this. Good to know there is some way of dealing with the title issue.

Glad to hear!

Quick question: what's the rationale for adding the hash of the file to the annotation? Not that I mind either way, just wondering.

The rationale is that the hash is unique and by using Some title [abcdef] as a key to find the buffer, we can be sufficiently certain that can deal with such duplicate titles since it results in an output like the following:

-----Org-roam-----
A test [5289ab7]  20221006154821-a_test.org
A test [ecf66d8]  20221006154803-a_test.org

(We could also use the datetime-prefix here, but since a hash is recorded in the DB I chose that.)

Of course! I was somehow expecting the titles to be 'uniquified' like buffers sometimes are, but this is probably a better option.

Now: would it be possible to make the hash be less prominent? In a way, it's not very helpful information to the user, but only something being used under the hood to go back and forth between titles and buffer names. Does that seem right to you?

I would propose to do it like so:

;; Find index to put
(setq idx
  (cl-position 'consult--source-buffer consult-buffer-sources :test 'equal))
;; Store the elements coming after the insertion point 
(setq tail (nthcdr idx consult-buffer-sources))
;; Insert element and set its cdr
(setcdr
  (nthcdr (1- idx) consult-buffer-sources)
  (append (list 'org-roam-buffer-source) tail))

Better solutions welcome.

That looks good enough to me!

So, do you think this new capability should be pushed as consult-org-roam-buffer.el?

That'd be great. Happy to write the PR if you want, or just let you do it yourself.

Best,

A.

Best regards, jgru

apc commented

Another thing: can you try deleting an org-roam file (using dired) that was open and see what happens if you call consult-buffer right afterwards?

(file-missing "Opening input file" "No such file or directory" "/Users/apc/org/notebook/varia/20221006133747.org")

On my end, it looks like the problem is that org-roam-db--file-hash is looking for a file listed in org-roam-open-buffer-lists and not finding it. This seems to be due to the fact that buffer-list will still output the name of the buffer corresponding to the file that no longer exists, and as a result so will org-roam-buffer-list.

Not sure what's best here: moving the call to (consult-org-roam-buffer--update-open-buffer-list) elsewhere or maybe changing the definition so that the function doesn't apply consult-org-roam-buffer--add-title to the entire list coming from org-roam-buffer-list but only to any item on that list that wasn't there last time the function was run.

Thoughts?

jgru commented

Thoughts?

Good catch! You are right org-roam-db--file-hash throws an error when the file has been deleted (or is otherwise inexistent).
The following implementation avoids the error:

(defun consult-org-roam-buffer--get-title (buffer)
  "Select from list of all notes that link to the current note."
  (if (org-roam-buffer-p buffer)
    (let* ((title
             (with-current-buffer buffer
               (org-roam-db--file-title)))
            (filename (buffer-file-name buffer))
            (fhash (consult-org-roam-db--file-hash filename)))
      (if fhash
        (concat title " [" (substring fhash 0 7) "]")
        (concat title " [not persisted]")))))

(defun consult-org-roam-db--file-hash (fname)
  "docstring"
  (let* ((fhash (org-roam-db-query
                  [:select [hash]
                    :from files
                    :where (like file $s1)
                    ]
                  (concat "%" fname))))
    (car (car fhash)))) 

I added this functionality as PR #13. Please a have look and let me know what you think.
It would be very helpful, if you could testdrive this in a plain new installation and see whether this works for you.

Thank you very much for working on this.

Best regards,
jgru