GNU bug report logs - #39722
Support for bookmark.el in VC directory buffers

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39722 in the body.
You can then email your comments to 39722 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Matthias Meulien <orontee <at> gmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: Support for bookmark.el in VC directory buffers
Date: Fri, 21 Feb 2020 22:19:34 +0100

[Message part 1 (text/plain, inline)]

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.
-- 
Matthias

[0001-Bookmark-locations-can-refer-to-VC-directory-buffers.patch (text/x-patch, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #12 received at 39722 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Matthias Meulien <orontee <at> gmail.com>
Cc: 39722 <at> debbugs.gnu.org
Subject: Re: bug#39722: Support for bookmark.el in VC directory buffers
Date: Wed, 13 May 2020 01:46:05 +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.

Thanks, this is a useful addition.  Please push it after fixing minor
details:

> +(defun vc-dir-bookmark-make-record ()
> +  "This implements the `bookmark-make-record-function' type for
> +`vc-dir' buffers."
> ...
> +(defun vc-dir-bookmark-jump (bmk)
> +  "This implements the `handler' function interface for the record
> +type returned by `vc-dir-bookmark-make-record'."

According to the documentation standard described in
(info "(elisp) Documentation Tips")
the first line of the docstring should consist of a complete sentence.

I see that you copied the docstring from Info-bookmark-jump,
but the docstrings of info.el contains a mistake
that should be fixed in info.el.

Message #15 received at 39722 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Meulien <orontee <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 39722 <at> debbugs.gnu.org
Subject: Re: bug#39722: Support for bookmark.el in VC directory buffers
Date: Wed, 13 May 2020 02:35:08 +0200

[Message part 1 (text/plain, inline)]

> (...) Thanks, this is a useful addition. Please push it after 
> fixing minor details: 

Thanks Juri for your remark. I updated the patch. Unfortunately I 
am not authorized to push.

> According to the documentation standard described in (info 
> "(elisp) Documentation Tips") the first line of the docstring 
> should consist of a complete sentence.

I'll keep this in mind.

[0001-Bookmark-locations-can-refer-to-VC-directory-buffers.patch (text/x-diff, inline)]

From 4d4d3e819dc849268e7b3a15ea0d2390c337c23c Mon Sep 17 00:00:00 2001
From: Matthias Meulien <orontee <at> gmail.com>
Date: Fri, 21 Feb 2020 22:13:48 +0100
Subject: [PATCH] Bookmark locations can refer to VC directory buffers

* etc/NEWS: Document feature
* lisp/vc/vc-dir.el (vc-dir-mode): Set local bookmark-make-record-function
(bookmark-make-record-default)
(bookmark-prop-get)
(bookmark-default-handler)
(bookmark-get-bookmark-record): Declarations
(vc-dir-bookmark-make-record): Make record used to bookmark a `vc-dir' buffer.
(vc-dir-bookmark-jump): Provides the bookmark-jump behavior for a `vc-dir' buffer.
---
 etc/NEWS          |  4 ++++
 lisp/vc/vc-dir.el | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index ae676a9bf8..01c584833b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -348,6 +348,10 @@ symbol property to the browsing functions.  With a new command
 'browse-url-with-browser-kind', an URL can explicitly be browsed with
 either an internal or external browser.
 
+** vc-dir.el
+
+*** Support for bookmark.el.
+Bookmark locations can refer to VC directory buffers.
 
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index 0c9e656add..a86c37c24a 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -1106,6 +1106,7 @@ vc-dir-mode
   (set (make-local-variable 'vc-dir-backend) use-vc-backend)
   (set (make-local-variable 'desktop-save-buffer)
        'vc-dir-desktop-buffer-misc-data)
+  (setq-local bookmark-make-record-function #'vc-dir-bookmark-make-record)
   (setq buffer-read-only t)
   (when (boundp 'tool-bar-map)
     (set (make-local-variable 'tool-bar-map) vc-dir-tool-bar-map))
@@ -1466,6 +1467,41 @@ vc-dir-restore-desktop-buffer
 	     '(vc-dir-mode . vc-dir-restore-desktop-buffer))
 
 
+;;; Support for bookmark.el (adapted from what info.el does).
+
+(declare-function bookmark-make-record-default
+                  "bookmark" (&optional no-file no-context posn))
+(declare-function bookmark-prop-get "bookmark" (bookmark prop))
+(declare-function bookmark-default-handler "bookmark" (bmk))
+(declare-function bookmark-get-bookmark-record "bookmark" (bmk))
+
+(defun vc-dir-bookmark-make-record ()
+  "Make record used to bookmark a `vc-dir' buffer.
+This implements the `bookmark-make-record-function' type for
+`vc-dir' buffers."
+  (let* ((bookmark-name
+          (concat "(" (symbol-name vc-dir-backend) ") "
+                  (file-name-nondirectory
+                   (directory-file-name default-directory))))
+         (defaults (list bookmark-name default-directory)))
+    `(,bookmark-name
+      ,@(bookmark-make-record-default 'no-file)
+      (filename . ,default-directory)
+      (handler . vc-dir-bookmark-jump)
+      (defaults . ,defaults))))
+
+;;;###autoload
+(defun vc-dir-bookmark-jump (bmk)
+  "Provides the bookmark-jump behavior for a `vc-dir' buffer.
+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)))))
+
+
 (provide 'vc-dir)
 
 ;;; vc-dir.el ends here
-- 
2.20.1

[Message part 3 (text/plain, inline)]

-- 
Matthias Meulien

Message #18 received at 39722 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Matthias Meulien <orontee <at> gmail.com>
Cc: 39722 <at> debbugs.gnu.org
Subject: Re: bug#39722: Support for bookmark.el in VC directory buffers
Date: Thu, 21 May 2020 01:38:49 +0300

tags 39722 fixed
close 39722 28.0.50
thanks

>> (...) Thanks, this is a useful addition. Please push it after fixing
>> minor details: 
>
> Thanks Juri for your remark. I updated the patch. Unfortunately I am not
> authorized to push.

Thanks for the patch, I pushed it to master.

Message #25 received at 39722 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Matthias Meulien <orontee <at> gmail.com>
Cc: 39722 <at> debbugs.gnu.org
Subject: Re: bug#39722: Support for bookmark.el in VC directory buffers
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))

Message #28 received at 39722 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Matthias Meulien <orontee <at> gmail.com>
Cc: 39722 <at> debbugs.gnu.org
Subject: Re: bug#39722: Support for bookmark.el in VC directory buffers
Date: Sun, 07 Jun 2020 02:52:21 +0300

[Message part 1 (text/plain, inline)]

> But I wonder what could be a proper solution?

I still have no idea how to avoid double call of display-buffer,
but at least here is the patch that avoids creating two tabs.
It creates a new tab only for the first call of display-buffer,
and resets display-buffer-overriding-action afterwards:

[debounce-display-buffer-override-next-command.patch (text/x-diff, inline)]

diff --git a/lisp/window.el b/lisp/window.el
index c047150413..2249b8d1d1 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8590,15 +8590,18 @@ display-buffer-override-next-command
   (let* ((old-window (or (minibuffer-selected-window) (selected-window)))
          (new-window nil)
          (minibuffer-depth (minibuffer-depth))
+         (clearfun (make-symbol "clear-display-buffer-overriding-action"))
          (action (lambda (buffer alist)
                    (unless (> (minibuffer-depth) minibuffer-depth)
                      (let* ((ret (funcall pre-function buffer alist))
                             (window (car ret))
                             (type (cdr ret)))
                        (setq new-window (window--display-buffer buffer window
-                                                                type alist))))))
+                                                                type alist))
+                       (funcall clearfun)
+                       (setq post-function nil)
+                       new-window))))
          (command this-command)
-         (clearfun (make-symbol "clear-display-buffer-overriding-action"))
          (exitfun
           (lambda ()
             (setq display-buffer-overriding-action
diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index a86c37c24a..e9ec22c86e 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -1496,8 +1496,9 @@ 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))
-         (buf (save-window-excursion
-                 (vc-dir file) (current-buffer))))
+         (buf (progn
+                (vc-dir file)
+                (current-buffer))))
     (bookmark-default-handler
      `("" (buffer . ,buf) . ,(bookmark-get-bookmark-record bmk)))))
 

Message #31 received at 39722 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Matthias Meulien <orontee <at> gmail.com>
Cc: 39722 <at> debbugs.gnu.org
Subject: Re: bug#39722: Support for bookmark.el in VC directory buffers
Date: Mon, 22 Jun 2020 02:36:33 +0300

>> But I wonder what could be a proper solution?
>
> I still have no idea how to avoid double call of display-buffer,
> but at least here is the patch that avoids creating two tabs.
> It creates a new tab only for the first call of display-buffer,
> and resets display-buffer-overriding-action afterwards:

Now pushed to master.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.