Subject: Re: bug#39722: Support for bookmark.el in VC directory buffers

From: Juri Linkov <[email-masked]>
Date: Fri, 05 Jun 2020 01:42:09 +0300

> Being able to bookmark VC directory buffers is useful when working on
> multiple projects; The proposed patch implements this on the lines of
> what I read in info.el.

> +(defun vc-dir-bookmark-jump (bmk)
> +  "This implements the `handler' function interface for the record
> +type returned by `vc-dir-bookmark-make-record'."
> +  (let* ((file (bookmark-prop-get bmk 'filename))
> +         (buf (save-window-excursion
> +                 (vc-dir file) (current-buffer))))
> +    (bookmark-default-handler
> +     `("" (buffer . ,buf) . ,(bookmark-get-bookmark-record bmk)))))

While using the new keybinding 'C-x t t' from bug#41691
(instead of adding a new command bookmark-jump-other-tab)
I noticed that vc-dir-bookmark-jump doesn't work with it nicely -
it creates two tabs instead of one new tab.  This is because
'C-x t t' modifies display-buffer-overriding-action
with a function that creates a new tab, but the problem is that
visiting a bookmark with a vc-dir buffer calls display-buffer
*twice*.  A new tab is created for every display-buffer call.

I see that you tried to implement a workaround for this problem
by using (save-window-excursion (vc-dir file) that handles some cases.

Another workaround could fix the tab problem:

diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index a86c37c24a..1c5be268f4 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -1496,8 +1496,10 @@ vc-dir-bookmark-jump
 This implements the `handler' function interface for the record
 type returned by `vc-dir-bookmark-make-record'."
   (let* ((file (bookmark-prop-get bmk 'filename))
+         (display-buffer-overriding-action '(nil))
          (buf (save-window-excursion
-                 (vc-dir file) (current-buffer))))
+                (vc-dir file)
+                (current-buffer))))
     (bookmark-default-handler
      `("" (buffer . ,buf) . ,(bookmark-get-bookmark-record bmk)))))

But I wonder what could be a proper solution?

The first call of pop-to-buffer is in 'vc-dir':

  (let (pop-up-windows)		      ; based on cvs-examine; bug#6204
    (pop-to-buffer (vc-dir-prepare-status-buffer "*vc-dir*" dir backend)))

(Maybe there should be a separate function 'vc-dir-no-select'
 that doesn't call pop-to-buffer?)

The second call is in bookmark-jump with:

  (bookmark--jump-via bookmark (or display-func 'pop-to-buffer-same-window))

Also in bookmark-bmenu-other-window:

    (bookmark--jump-via bookmark 'switch-to-buffer-other-window)

And in bookmark-bmenu-switch-other-window:

  (let ((bookmark (bookmark-bmenu-bookmark))
	(fun (lambda (b) (display-buffer b t))))
    (bookmark--jump-via bookmark fun))