Bookmarks with handlers can eventually be valid directories

[nbarrientos]

Hi,

I see that https://github.com/karthink/consult-dir/commit/d3bb96abb5ccca29f4b04c6f623818386167a2b2 added a constraint so bookmarks with a handler like:

("ndev9al"
  (handler . eshell-bookmark--restore)
  (filename .
            #("/ssh:ndev9al.example.org:/" 5 20
              (escaped t))))

are be excluded when calling consult-dir--bookmark-dirs. However, as you can see in the example above, that bookmark is still a valid directory. If I remove the part that ejects candidates with a handler:

      (cl-remove-if     (lambda (cand) (bookmark-get-handler cand)))

The bookmark is available (as it should I think) when I call consult-dir.

Is there any reason for excluding all bookmarks with handlers? Could this be improved so eshell bookmarks can be used as candidates?

Something like this could work for me but dunno if it's the best approach, to be honest.

@@ -206,7 +206,7 @@ defun consult-dir--bookmark-dirs
   (bookmark-maybe-load-default-file)
   (let ((file-narrow ?f))
     (thread-last bookmark-alist
-      (cl-remove-if     (lambda (cand) (bookmark-get-handler cand)))
+      (cl-remove-if     (lambda (cand) (not (member (bookmark-get-handler cand) '(eshell-bookmark--restore)))))
       (cl-remove-if-not (lambda (cand)
                           (let ((bm (bookmark-get-bookmark-record cand)))
                             (when-let ((file (alist-get 'filename bm)))

Thanks.

[karthink]

Since the bookmark system is open-ended, it's not clear how to figure out if a bookmark is eventually a directory. Bookmarks can be anything from a web link to a frame configuration to executable elisp, and there's no prescribed structure to the entires in bookmark-alist.

So consult-dir does the safe thing and only shows bookmarks that are explicitly directories. If you want to include special bookmark types in consult-dir, you can do so by adding a directory source that finds the directories corresponding to these bookmarks.

[nbarrientos]

Thanks for your message. I understand, but what about removing:

(cl-remove-if     (lambda (cand) (bookmark-get-handler cand)))

and relying only on checking if file-directory-p is t for the filename key of the bookmark if it exists (as you're currently doing)? Ain't this check on its own good enough to select directories for all types of bookmarks?

In other words, if this patch was applied:

@@ -206,7 +206,6 @@ defun consult-dir--bookmark-dirs
   (bookmark-maybe-load-default-file)
   (let ((file-narrow ?f))
     (thread-last bookmark-alist
-      (cl-remove-if     (lambda (cand) (bookmark-get-handler cand)))
       (cl-remove-if-not (lambda (cand)
                           (let ((bm (bookmark-get-bookmark-record cand)))
                             (when-let ((file (alist-get 'filename bm)))

what type of non-directory bookmarks would be erroneously returned by consult-dir--default-dirs?