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

Rough idea: add source for consult-buffer #12

Closed
apc opened this issue Oct 5, 2022 · 7 comments · Fixed by #13
Closed

Rough idea: add source for consult-buffer #12

apc opened this issue Oct 5, 2022 · 7 comments · Fixed by #13
Labels
enhancement New feature or request

Comments

@apc
Copy link

apc commented Oct 5, 2022

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
Copy link
Owner

jgru commented Oct 6, 2022

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
Copy link
Author

apc commented Oct 6, 2022

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
Copy link
Author

apc commented Oct 6, 2022

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
Copy link
Owner

jgru commented Oct 6, 2022

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
Copy link
Author

apc commented Oct 6, 2022

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
Copy link
Author

apc commented Oct 6, 2022

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 added a commit that referenced this issue Oct 7, 2022
Add consult-org-roam-buffer to fix #12
jgru added a commit that referenced this issue Oct 7, 2022
Add consult-org-roam-buffer to fix #12
@jgru
Copy link
Owner

jgru commented Oct 7, 2022

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

@jgru jgru added the enhancement New feature or request label Oct 7, 2022
@jgru jgru linked a pull request Oct 7, 2022 that will close this issue
@jgru jgru closed this as completed in #13 Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants