bug#41691: [PATCH] Add bookmark-jump-other-tab

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Javier Olaechea]

* lisp/bookmark.el (bookmark-jump-other-tab): Add function to open a
bookmark on another tab
---
 lisp/bookmark.el | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 5bb1698171..99bb70ae0e 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -1142,6 +1142,14 @@ bookmark-jump-other-frame
   (let ((pop-up-frames t))
     (bookmark-jump-other-window bookmark)))
 
+;;;###autoload
+(defun bookmark-jump-other-tab (bookmark)
+  "Jump to BOOKMARK in another tab.  See `bookmark-jump' for more."
+  (interactive
+   (list (bookmark-completing-read "Jump to bookmark (in another tab)"
+                                   bookmark-current-bookmark)))
+  (bookmark-jump bookmark 'switch-to-buffer-other-tab))
+
 (defun bookmark-jump-noselect (bookmark)
   "Return the location pointed to by BOOKMARK (see `bookmark-jump').
 The return value has the form (BUFFER . POINT).
-- 
2.17.1

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Glenn Morris]

Personally I hope that every "-other-window" command doesn't get an
"-other-tab" counterpart (to go along with "-other-frame"), but rather
that a single systematic method is developed, like the elpa.gnu.org
package other-frame-window. Ref eg

https://lists.gnu.org/r/emacs-devel/2019-10/msg00261.html

(AFAICS nothing has been implemented)

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Juri Linkov]

> Personally I hope that every "-other-window" command doesn't get an
> "-other-tab" counterpart (to go along with "-other-frame"), but rather
> that a single systematic method is developed, like the elpa.gnu.org
> package other-frame-window. Ref eg
>
> https://lists.gnu.org/r/emacs-devel/2019-10/msg00261.html
>
> (AFAICS nothing has been implemented)

Actually, this is already implemented, but without an obvious keybinding.
This small patch binds a general command to 'C-x t t' (or maybe a better
key would be 'C-x t w').

Then after typing 'C-x t t' as a prefix key sequence,
the next command will display its buffer in a new tab:

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 76e7f8c33a..529f5e5e4b 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -1585,6 +1585,7 @@ tab-prefix-map
 (define-key tab-prefix-map "b" 'switch-to-buffer-other-tab)
 (define-key tab-prefix-map "f" 'find-file-other-tab)
 (define-key tab-prefix-map "\C-f" 'find-file-other-tab)
+(define-key tab-prefix-map "t" 'windmove-display-new-tab)
 
 \f
 (provide 'tab-bar)

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

>> Personally I hope that every "-other-window" command doesn't get an
>> "-other-tab" counterpart (to go along with "-other-frame"), but rather
>> that a single systematic method is developed, like the elpa.gnu.org
>> package other-frame-window. Ref eg
>>
>> https://lists.gnu.org/r/emacs-devel/2019-10/msg00261.html
>>
>> (AFAICS nothing has been implemented)
>
> Actually, this is already implemented, but without an obvious keybinding.
> This small patch binds a general command to 'C-x t t' (or maybe a better
> key would be 'C-x t w').
>
> Then after typing 'C-x t t' as a prefix key sequence,
> the next command will display its buffer in a new tab:
>
> @@ -1585,6 +1585,7 @@ tab-prefix-map
>  (define-key tab-prefix-map "b" 'switch-to-buffer-other-tab)
>  (define-key tab-prefix-map "f" 'find-file-other-tab)
>  (define-key tab-prefix-map "\C-f" 'find-file-other-tab)
> +(define-key tab-prefix-map "t" 'windmove-display-new-tab)

I see that using windmove-display-new-tab in tab-bar.el is inappropriate,
so here is refactoring:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: other-tab-prefix.patch --]
[-- Type: text/x-diff, Size: 8667 bytes --]

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 76e7f8c33a..dd9d520ed9 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -1575,6 +1575,21 @@ find-file-other-tab
           value)
       (switch-to-buffer-other-tab value))))
 
+(defun other-tab-prefix ()
+  "Display the next buffer in a new tab.
+The next buffer is the buffer displayed by the next command invoked
+immediately after this command (ignoring reading from the minibuffer).
+Creates a new tab before displaying the buffer.
+When `switch-to-buffer-obey-display-actions' is non-nil,
+`switch-to-buffer' commands are also supported."
+  (interactive)
+  (display-buffer-override-next-command
+   (lambda (buffer alist)
+     (cons (let ((tab-bar-new-tab-choice t))
+             (tab-bar-new-tab)
+             (selected-window))
+           'tab))))
+
 (define-key tab-prefix-map "2" 'tab-new)
 (define-key tab-prefix-map "1" 'tab-close-other)
 (define-key tab-prefix-map "0" 'tab-close)
@@ -1585,6 +1600,7 @@ tab-prefix-map
 (define-key tab-prefix-map "b" 'switch-to-buffer-other-tab)
 (define-key tab-prefix-map "f" 'find-file-other-tab)
 (define-key tab-prefix-map "\C-f" 'find-file-other-tab)
+(define-key tab-prefix-map "t" 'other-tab-prefix)
 
 \f
 (provide 'tab-bar)
diff --git a/lisp/window.el b/lisp/window.el
index d658cb81f6..3f851c86f4 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -10069,6 +10069,46 @@ window--adjust-process-windows
                 (set-process-window-size process (cdr size) (car size))))))))))
 
 (add-hook 'window-configuration-change-hook 'window--adjust-process-windows)
+
+\f
+(defun display-buffer-override-next-command (action-function &optional exit-function)
+  "Override `display-buffer-overriding-action' for the next command.
+`action-function' is called to prepare the window where the buffer should
+be displayed.  This function takes two arguments `buffer' and `alist', and
+should return a cons with the displayed window and type.  See argument
+meaning in `window--display-buffer'.  `exit-function' is called after the
+buffer is displayed in the window.  The function takes one argument `window'."
+  (let* ((new-window)
+         (minibuffer-depth (minibuffer-depth))
+         (action (lambda (buffer alist)
+                   (unless (> (minibuffer-depth) minibuffer-depth)
+                     (let* ((ret (funcall action-function buffer alist))
+                            (window (car ret))
+                            (type (cdr ret)))
+                       (setq new-window (window--display-buffer buffer window
+                                                                type alist))))))
+         (command this-command)
+         (clearfun (make-symbol "clear-display-buffer-overriding-action"))
+         (exitfun
+          (lambda ()
+            (setq display-buffer-overriding-action
+                  (delq action display-buffer-overriding-action))
+            (when (functionp exit-function)
+              (funcall exit-function new-window))
+            (remove-hook 'post-command-hook clearfun))))
+    (fset clearfun
+          (lambda ()
+            (unless (or
+		     ;; Remove the hook immediately
+		     ;; after exiting the minibuffer.
+		     (> (minibuffer-depth) minibuffer-depth)
+		     ;; But don't remove immediately after
+		     ;; adding the hook by the same command below.
+		     (eq this-command command))
+              (funcall exitfun))))
+    (add-hook 'post-command-hook clearfun)
+    (push action display-buffer-overriding-action)))
+
 \f
 ;; Some of these are in tutorial--default-keys, so update that if you
 ;; change these.
diff --git a/lisp/windmove.el b/lisp/windmove.el
index f96383197b..38e3fee83a 100644
--- a/lisp/windmove.el
+++ b/lisp/windmove.el
@@ -462,59 +462,38 @@ windmove-display-in-direction
 When `switch-to-buffer-obey-display-actions' is non-nil,
 `switch-to-buffer' commands are also supported."
   (let* ((no-select (xor (consp arg) windmove-display-no-select))
-         (old-window (or (minibuffer-selected-window) (selected-window)))
-         (new-window)
-         (minibuffer-depth (minibuffer-depth))
-         (action (lambda (buffer alist)
-                   (unless (> (minibuffer-depth) minibuffer-depth)
-                     (let* ((type 'reuse)
-                            (window (cond
-                                     ((eq dir 'new-tab)
-                                      (let ((tab-bar-new-tab-choice t))
-                                        (tab-bar-new-tab))
-                                      (setq type 'tab)
-                                      (selected-window))
-                                     ((eq dir 'new-frame)
-                                      (let* ((params (cdr (assq 'pop-up-frame-parameters alist)))
-                                             (pop-up-frame-alist (append params pop-up-frame-alist))
-                                             (frame (make-frame-on-current-monitor
-                                                     pop-up-frame-alist)))
-                                        (unless (cdr (assq 'inhibit-switch-frame alist))
-	                                  (window--maybe-raise-frame frame))
-                                        (setq type 'frame)
-                                        (frame-selected-window frame)))
-                                     ((eq dir 'same-window)
-                                      (selected-window))
-                                     (t (window-in-direction
-                                         dir nil nil
-                                         (and arg (prefix-numeric-value arg))
-                                         windmove-wrap-around)))))
-                       (unless window
-                         (setq window (split-window nil nil dir) type 'window))
-                       (setq new-window (window--display-buffer buffer window
-                                                                type alist))))))
-         (command this-command)
-         (clearfun (make-symbol "clear-display-buffer-overriding-action"))
-         (exitfun
-          (lambda ()
-            (setq display-buffer-overriding-action
-                  (delq action display-buffer-overriding-action))
-            (when (window-live-p (if no-select old-window new-window))
-              (select-window (if no-select old-window new-window)))
-            (remove-hook 'post-command-hook clearfun))))
-    (fset clearfun
-          (lambda ()
-            (unless (or
-		     ;; Remove the hook immediately
-		     ;; after exiting the minibuffer.
-		     (> (minibuffer-depth) minibuffer-depth)
-		     ;; But don't remove immediately after
-		     ;; adding the hook by the same command below.
-		     (eq this-command command))
-              (funcall exitfun))))
-    (add-hook 'post-command-hook clearfun)
-    (push action display-buffer-overriding-action)
-    (message "[display-%s]" dir)))
+         (old-window (or (minibuffer-selected-window) (selected-window))))
+    (display-buffer-override-next-command
+     (lambda (buffer alist)
+       (let* ((type 'reuse)
+              (window (cond
+                       ((eq dir 'new-tab)
+                        (let ((tab-bar-new-tab-choice t))
+                          (tab-bar-new-tab))
+                        (setq type 'tab)
+                        (selected-window))
+                       ((eq dir 'new-frame)
+                        (let* ((params (cdr (assq 'pop-up-frame-parameters alist)))
+                               (pop-up-frame-alist (append params pop-up-frame-alist))
+                               (frame (make-frame-on-current-monitor
+                                       pop-up-frame-alist)))
+                          (unless (cdr (assq 'inhibit-switch-frame alist))
+	                    (window--maybe-raise-frame frame))
+                          (setq type 'frame)
+                          (frame-selected-window frame)))
+                       ((eq dir 'same-window)
+                        (selected-window))
+                       (t (window-in-direction
+                           dir nil nil
+                           (and arg (prefix-numeric-value arg))
+                           windmove-wrap-around)))))
+         (unless window
+           (setq window (split-window nil nil dir) type 'window))
+         (cons window type)))
+     (lambda (new-window)
+       (when (window-live-p (if no-select old-window new-window))
+         (select-window (if no-select old-window new-window))))))
+  (message "[display-%s]" dir))
 
 ;;;###autoload
 (defun windmove-display-left (&optional arg)

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Juri Linkov]

>> Then after typing 'C-x t t' as a prefix key sequence,
>> the next command will display its buffer in a new tab:

Now pushed to master.

Javier, do you agree this is a good way to open a bookmark
on another tab with an easy-to-type key sequence:

  C-x t t
  C-x r b

so this feature request can be closed?

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Javier Olaechea]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

> do you agree this is a good way to open a bookmark
on another tab with an easy-to-type key sequence:

Yeah. It is better to provide a general solution.

On Sat, Jun 6, 2020 at 6:54 PM Juri Linkov <juri@linkov.net> wrote:

> >> Then after typing 'C-x t t' as a prefix key sequence,
> >> the next command will display its buffer in a new tab:
>
> Now pushed to master.
>
> Javier, do you agree this is a good way to open a bookmark
> on another tab with an easy-to-type key sequence:
>
>   C-x t t
>   C-x r b
>
> so this feature request can be closed?
>


-- 
"I object to doing things that computers can do." — Olin Shivers

[-- Attachment #2: Type: text/html, Size: 1079 bytes --]

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 221 bytes --]

>> Then after typing 'C-x t t' as a prefix key sequence,
>> the next command will display its buffer in a new tab:

Now here is a patch for doing the same for a new window with
'C-x 4 4', and a new frame with 'C-x 5 5':


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: other-window-frame.patch --]
[-- Type: text/x-diff, Size: 2935 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index b6f44341e8..c3d1a775b7 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4005,6 +4005,22 @@ other-window
 	;; Always return nil.
 	nil))))
 
+(defun other-window-prefix ()
+  "Display the buffer of the next command in a new window.
+The next buffer is the buffer displayed by the next command invoked
+immediately after this command (ignoring reading from the minibuffer).
+Creates a new window before displaying the buffer, or switches to the window
+that already contains that buffer.
+When `switch-to-buffer-obey-display-actions' is non-nil,
+`switch-to-buffer' commands are also supported."
+  (interactive)
+  (display-buffer-override-next-command
+   (lambda (buffer alist)
+     (cons (display-buffer-pop-up-window
+            buffer (append alist '((inhibit-same-window . t))))
+           'window)))
+  (message "Display next command buffer in a new window..."))
+
 ;; This should probably return non-nil when the selected window is part
 ;; of an atomic window whose root is the frame's root window.
 (defun one-window-p (&optional nomini all-frames)
@@ -10131,5 +10147,6 @@ ctl-x-map
 (define-key ctl-x-map "-" 'shrink-window-if-larger-than-buffer)
 (define-key ctl-x-map "+" 'balance-windows)
 (define-key ctl-x-4-map "0" 'kill-buffer-and-window)
+(define-key ctl-x-4-map "4" 'other-window-prefix)
 
 ;;; window.el ends here
diff --git a/lisp/frame.el b/lisp/frame.el
index 6c2f774709..25634fb445 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1070,6 +1070,23 @@ other-frame
       (setq arg (1+ arg)))
     (select-frame-set-input-focus frame)))
 
+(defun other-frame-prefix ()
+  "Display the buffer of the next command in a new frame.
+The next buffer is the buffer displayed by the next command invoked
+immediately after this command (ignoring reading from the minibuffer).
+Creates a new frame before displaying the buffer, or switches to the frame
+that already contains that buffer.
+When `switch-to-buffer-obey-display-actions' is non-nil,
+`switch-to-buffer' commands are also supported."
+  (interactive)
+  (display-buffer-override-next-command
+   (lambda (buffer alist)
+     (cons (display-buffer-pop-up-frame
+            buffer (append alist '((reusable-frames . 0)
+                                   (inhibit-same-window . t))))
+           'frame)))
+  (message "Display next command buffer in a new frame..."))
+
 (defun iconify-or-deiconify-frame ()
   "Iconify the selected frame, or deiconify if it's currently an icon."
   (interactive)
@@ -2697,6 +2714,7 @@ ctl-x-5-map
 (define-key ctl-x-5-map "1" 'delete-other-frames)
 (define-key ctl-x-5-map "0" 'delete-frame)
 (define-key ctl-x-5-map "o" 'other-frame)
+(define-key ctl-x-5-map "5" 'other-frame-prefix)
 (define-key global-map [f11] 'toggle-frame-fullscreen)
 (define-key global-map [(meta f10)] 'toggle-frame-maximized)
 (define-key esc-map    [f10]        'toggle-frame-maximized)

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Juri Linkov]

>>> Then after typing 'C-x t t' as a prefix key sequence,
>>> the next command will display its buffer in a new tab:
>
> Now here is a patch for doing the same for a new window with
> 'C-x 4 4', and a new frame with 'C-x 5 5':

Pushed to master.  There is also a new command 'same-window-prefix',
but it's currently unbound.  What would be a good keybinding?
Maybe 'C-x 4 1'?

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Mon, 22 Jun 2020 02:18:27 +0300
> 
> > Now here is a patch for doing the same for a new window with
> > 'C-x 4 4', and a new frame with 'C-x 5 5':
> 
> Pushed to master.  There is also a new command 'same-window-prefix',
> but it's currently unbound.  What would be a good keybinding?
> Maybe 'C-x 4 1'?

Please be sure to call out these changes in NEWS, and possibly also in
the ELisp manual.

Thanks.

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Juri Linkov]

>> > Now here is a patch for doing the same for a new window with
>> > 'C-x 4 4', and a new frame with 'C-x 5 5':
>> 
>> Pushed to master.  There is also a new command 'same-window-prefix',
>> but it's currently unbound.  What would be a good keybinding?
>> Maybe 'C-x 4 1'?
>
> Please be sure to call out these changes in NEWS, and possibly also in
> the ELisp manual.

These changes were already in NEWS, but other details are still
under discussion on emacs-devel.

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 41691@debbugs.gnu.org
> Date: Mon, 29 Jun 2020 01:00:01 +0300
> 
> >> > Now here is a patch for doing the same for a new window with
> >> > 'C-x 4 4', and a new frame with 'C-x 5 5':
> >> 
> >> Pushed to master.  There is also a new command 'same-window-prefix',
> >> but it's currently unbound.  What would be a good keybinding?
> >> Maybe 'C-x 4 1'?
> >
> > Please be sure to call out these changes in NEWS, and possibly also in
> > the ELisp manual.
> 
> These changes were already in NEWS

IMNSHO, insufficiently so.  The Lisp-level aspects of this are not
described, AFAICT, and the fact that this is a prefix command isn't
explained.  This is going to be a major feature (or at least it can
be), so a single sentence is barely enough to describe it in NEWS.

Can we please improve the description of these new features?

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Juri Linkov]

>> > Please be sure to call out these changes in NEWS, and possibly also in
>> > the ELisp manual.
>>
>> These changes were already in NEWS
>
> IMNSHO, insufficiently so.  The Lisp-level aspects of this are not

These commands are not intended to be used programmatically at Lisp-level.
Or do you mean to describe a new function 'display-buffer-override-next-command'?

> described, AFAICT, and the fact that this is a prefix command isn't

NEWS mentions the key prefix.  Do you think NEWS should also mention
the name of the prefix command like 'other-window-prefix'?

> explained.  This is going to be a major feature (or at least it can
> be), so a single sentence is barely enough to describe it in NEWS.

Do you want to copy the docstrings of these commands to NEWS?

> Can we please improve the description of these new features?

Definitely they will be described in the manual soon.

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Juri Linkov]

> Please be sure to call out these changes in NEWS, and possibly also in
> the ELisp manual.

Please check if it's better now.

bug#41691: [PATCH] Add bookmark-jump-other-tab

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 41691@debbugs.gnu.org
> Date: Wed, 01 Jul 2020 00:30:47 +0300
> 
> > Please be sure to call out these changes in NEWS, and possibly also in
> > the ELisp manual.
> 
> Please check if it's better now.

Much better, thank you.