unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#70819: 29.3; clone-indirect-buffer-other-window is inconsistent with *-other-window commands
@ 2024-05-07 16:19 Richard Sent
  2024-05-11  9:29 ` Eli Zaretskii
  2024-05-27 15:32 ` bug#70819: [PATCH v2] Make clone-indirect-buffer-other-window use other window (bug#70819) Richard Sent
  0 siblings, 2 replies; 13+ messages in thread
From: Richard Sent @ 2024-05-07 16:19 UTC (permalink / raw)
  To: 70819

Hi Emacs!

clone-indirect-buffer-other-window does not match the behavior
*-other-window commands (e.g. find-file-other-window).

find-file-other-window calls switch-to-buffer-other-window, which
ensures that the new file /always/ appears in a different window than
the current one.

clone-indirect-buffer-other-window insteads wraps clone-indirect-buffer
in a (pop-up-windows t) binding, which (somehow) results in different
behavior. Depending on the value of display-buffer-alist, the "other
window" can in fact be the active window, making
clone-indirect-buffer-other-window behave indentically to
clone-indirect-buffer.

This can be reproduced with Emacs -Q and the minimal config:

--8<---------------cut here---------------start------------->8---
(add-to-list 'display-buffer-alist
               '(".*"
                 (display-buffer-reuse-window display-buffer-same-window))
               ;; Append so default behavior is lowest priority
               t)
--8<---------------cut here---------------end--------------->8---

The convention with *-other-window commands is to, well, run * and
display in another window, regardless of the settings in
display-buffer-alist. switch-to-buffer-other-window is another example
of a command that follows the expected behavior, even though it follows
a similar pattern to clone-indirect-buffer-other-window (wrap a command
in (pop-up-windows t)).

As a possible solution I found that this replacement for
clone-indirect-buffer-other-window behaves as expected:

--8<---------------cut here---------------start------------->8---
(defun my-clone-indirect-buffer-other-window (newname display-flag &optional norecord)
  "Like `clone-indirect-buffer' but display in another window."
  (interactive
   (progn
     (if (get major-mode 'no-clone-indirect)
	 (error "Cannot indirectly clone a buffer in %s mode" mode-name))
     (list (if current-prefix-arg
	       (read-buffer "Name of indirect buffer: " (current-buffer)))
	   t)))
  (switch-to-buffer-other-window
   (clone-indirect-buffer newname display-flag norecord)))
--8<---------------cut here---------------end--------------->8---


In GNU Emacs 29.3 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.41,
cairo version 1.18.0)
Windowing system distributor 'The X.Org Foundation', version 11.0.12101012
System Description: Guix System

Configured using:
 'configure
 CONFIG_SHELL=/gnu/store/rib9g2ig1xf3kclyl076w28parmncg4k-bash-minimal-5.1.16/bin/bash
 SHELL=/gnu/store/rib9g2ig1xf3kclyl076w28parmncg4k-bash-minimal-5.1.16/bin/bash
 --prefix=/gnu/store/pa2xpl78avk687hk82601rdqiacrzip3-emacs-29.3
 --enable-fast-install --with-cairo --with-modules
 --with-native-compilation=aot --disable-build-details'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3
THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER X11 XDBE XIM XINPUT2 XPM
GTK3 ZLIB

Important settings:
  value of $EMACSLOADPATH: /home/richard/.guix-home/profile/share/emacs/site-lisp:/gnu/store/pa2xpl78avk687hk82601rdqiacrzip3-emacs-29.3/share/emacs/29.3/lisp
  value of $EMACSNATIVELOADPATH: /home/richard/.guix-home/profile/lib/emacs/native-site-lisp
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: Message

Minor modes in effect:
  beacon-mode: t
  all-the-icons-completion-mode: t
  which-key-mode: t
  display-time-mode: t
  marginalia-mode: t
  savehist-mode: t
  vertico-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  eat-eshell-mode: t
  shell-dirtrack-mode: t
  server-mode: t
  global-auto-revert-mode: t
  delete-selection-mode: t
  display-battery-mode: t
  override-global-mode: t
  mml-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: message-do-auto-fill
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  abbrev-mode: t

Load-path shadows:
/gnu/store/v8r6az9568lv4p8srgamrmsm92krn130-emacs-transient-0.6.0/share/emacs/site-lisp/transient-0.6.0/transient hides /gnu/store/pa2xpl78avk687hk82601rdqiacrzip3-emacs-29.3/share/emacs/29.3/lisp/transient
/gnu/store/w89y5r65d5d0gfp1pv522ylyfmhh0iv2-emacs-xref-1.6.3/share/emacs/site-lisp/xref-1.6.3/xref hides /gnu/store/pa2xpl78avk687hk82601rdqiacrzip3-emacs-29.3/share/emacs/29.3/lisp/progmodes/xref
/gnu/store/ilfyjbpfsc1lbqwyllx0kzqg9h31zic3-emacs-project-0.10.0/share/emacs/site-lisp/project-0.10.0/project hides /gnu/store/pa2xpl78avk687hk82601rdqiacrzip3-emacs-29.3/share/emacs/29.3/lisp/progmodes/project
/gnu/store/y42sbhcwppj5wzfd2cn4kwjpni301psh-emacs-soap-client-3.2.3/share/emacs/site-lisp/soap-client-3.2.3/soap-inspect hides /gnu/store/pa2xpl78avk687hk82601rdqiacrzip3-emacs-29.3/share/emacs/29.3/lisp/net/soap-inspect
/gnu/store/y42sbhcwppj5wzfd2cn4kwjpni301psh-emacs-soap-client-3.2.3/share/emacs/site-lisp/soap-client-3.2.3/soap-client hides /gnu/store/pa2xpl78avk687hk82601rdqiacrzip3-emacs-29.3/share/emacs/29.3/lisp/net/soap-client

Features:
(shadow sort emacsbug ace-window avy vc-git misearch multi-isearch
cl-print dabbrev embark-consult consult magit-bookmark bookmark
jka-compr shortdoc mule-util mail-extr embark-org embark ffap cal-iso
face-remap org-agenda org-element org-persist xdg org-id avl-tree
org-refile smartparens-org ob-plantuml org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-src ob-comint org-pcomplete org-list
org-footnote org-faces org-entities ob-emacs-lisp ob-core ob-eval
org-cycle org-table org-keys oc org-loaddefs find-func cal-menu calendar
cal-loaddefs ol org-fold org-fold-core org-compat org-version org-macs
smartparens-config smartparens-text smartparens rainbow-delimiters
hl-line rs-utils rs-ui beacon all-the-icons-completion all-the-icons
all-the-icons-faces data-material data-weathericons data-octicons
data-fileicons data-faicons data-alltheicons which-key diminish
doom-dracula-theme doom-themes doom-themes-base moody time rs-tools
daemons rs-smtp smtpmail rs-skeleton rs-navigation marginalia orderless
savehist vertico rs-media rs-magit magit-extras magit-submodule
magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull
magit-fetch magit-clone magit-remote magit-commit magit-sequence
magit-notes magit-worktree magit-tag magit-merge magit-branch
magit-reset magit-files magit-refs magit-status magit package
url-handlers magit-repos magit-apply magit-wip magit-log which-func
imenu magit-diff smerge-mode diff diff-mode git-commit log-edit
pcvs-util add-log magit-core magit-autorevert magit-margin
magit-transient magit-process with-editor magit-mode magit-git
magit-base magit-section cursor-sensor crm rs-integrations debbugs
soap-client url-http url-auth url-gw nsm rng-xsd rng-dt rng-util
xsd-regexp rs-ibuffer rs-guix rs-project rs-eshell esh-var esh-mode
esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups
esh-util eat term disp-table ehelp shell pcomplete color rs-erc rs-epg
rs-elfeed rs-editing rg files-x vc vc-dispatcher rg-info-hack advice
rg-menu rg-ibuffer rg-result wgrep-rg rg-history rg-header ibuf-ext
ibuffer ibuffer-loaddefs cus-edit wgrep grep rs-dired ls-lisp dired-x
rs-core server autorevert filenotify delsel comp comp-cstr warnings rx
exec-path-from-shell rs-constants battery dbus rs-auth-source
auth-source-pass rs-scheme skeleton geiser-guile info-look info
transient format-spec geiser geiser-debug geiser-repl geiser-image
geiser-capf geiser-doc geiser-menu geiser-autodoc geiser-edit
geiser-completion geiser-eval geiser-connection tq geiser-syntax scheme
geiser-impl help-fns radix-tree geiser-log geiser-popup view
geiser-custom geiser-base rs-rust rs-password-store rs-org plantuml-mode
xml dash edmacro kmacro use-package-bind-key bind-key rs-docker
rs-csharp rs-cl slime easy-mmode apropos compile etags fileloop
generator xref project arc-mode archive-mode noutline outline icons pp
comint ansi-osc ansi-color ring hyperspec thingatpt browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util url-parse auth-source eieio
eieio-core json map byte-opt url-vars rs-nnrss rs-gnus nnmail gnus-int
mail-source gnus-range message sendmail mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus nnheader
gnus-util text-property-search time-date subr-x mail-utils range mm-util
mail-prsvr wid-edit cus-load no-littering compat cl-macs gv cl-extra
help-mode cl-seq use-package-core cl-loaddefs cl-lib bytecomp
byte-compile lorem-ipsum-autoloads geiser-autoloads
geiser-guile-autoloads xterm-color-autoloads rust-mode-autoloads
xref-autoloads project-autoloads spinner-autoloads hydra-autoloads
ht-autoloads lsp-mode-autoloads flycheck-autoloads rustic-autoloads
password-store-autoloads pass-autoloads plantuml-mode-autoloads
yaml-mode-autoloads docker-compose-mode-autoloads
dockerfile-mode-autoloads web-mode-autoloads macrostep-autoloads
slime-autoloads which-key-autoloads moody-autoloads
doom-themes-autoloads shrink-path-autoloads nerd-icons-autoloads
doom-modeline-autoloads diminish-autoloads beacon-autoloads
all-the-icons-completion-autoloads memoize-autoloads
all-the-icons-autoloads daemons-autoloads embark-autoloads
consult-autoloads marginalia-autoloads orderless-autoloads
vertico-autoloads popup-autoloads f-autoloads dumb-jump-autoloads
avy-autoloads ace-window-autoloads tablist-autoloads pdf-tools-autoloads
async-autoloads transient-autoloads magit-autoloads
soap-client-autoloads debbugs-autoloads
eshell-syntax-highlighting-autoloads eat-autoloads elfeed-autoloads
s-autoloads rg-autoloads wgrep-autoloads rainbow-delimiters-autoloads
markdown-mode-autoloads dash-autoloads smartparens-autoloads
dirvish-autoloads exec-path-from-shell-autoloads compat-autoloads
no-littering-autoloads guix-emacs rmc iso-transl tooltip cconv eldoc
paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 528988 94652)
 (symbols 48 37661 19)
 (strings 32 143315 8616)
 (string-bytes 1 5332872)
 (vectors 16 74407)
 (vector-slots 8 1234663 74885)
 (floats 8 1065 1349)
 (intervals 56 3951 475)
 (buffers 984 18))

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: 29.3; clone-indirect-buffer-other-window is inconsistent with *-other-window commands
  2024-05-07 16:19 bug#70819: 29.3; clone-indirect-buffer-other-window is inconsistent with *-other-window commands Richard Sent
@ 2024-05-11  9:29 ` Eli Zaretskii
  2024-05-12  8:30   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-13  2:26   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-27 15:32 ` bug#70819: [PATCH v2] Make clone-indirect-buffer-other-window use other window (bug#70819) Richard Sent
  1 sibling, 2 replies; 13+ messages in thread
From: Eli Zaretskii @ 2024-05-11  9:29 UTC (permalink / raw)
  To: Richard Sent, martin rudalics, Stefan Monnier; +Cc: 70819

> From: Richard Sent <richard@freakingpenguin.com>
> Date: Tue, 07 May 2024 12:19:27 -0400
> 
> clone-indirect-buffer-other-window does not match the behavior
> *-other-window commands (e.g. find-file-other-window).
> 
> find-file-other-window calls switch-to-buffer-other-window, which
> ensures that the new file /always/ appears in a different window than
> the current one.
> 
> clone-indirect-buffer-other-window insteads wraps clone-indirect-buffer
> in a (pop-up-windows t) binding, which (somehow) results in different
> behavior. Depending on the value of display-buffer-alist, the "other
> window" can in fact be the active window, making
> clone-indirect-buffer-other-window behave indentically to
> clone-indirect-buffer.
> 
> This can be reproduced with Emacs -Q and the minimal config:
> 
> --8<---------------cut here---------------start------------->8---
> (add-to-list 'display-buffer-alist
>                '(".*"
>                  (display-buffer-reuse-window display-buffer-same-window))
>                ;; Append so default behavior is lowest priority
>                t)
> --8<---------------cut here---------------end--------------->8---
> 
> The convention with *-other-window commands is to, well, run * and
> display in another window, regardless of the settings in
> display-buffer-alist. switch-to-buffer-other-window is another example
> of a command that follows the expected behavior, even though it follows
> a similar pattern to clone-indirect-buffer-other-window (wrap a command
> in (pop-up-windows t)).
> 
> As a possible solution I found that this replacement for
> clone-indirect-buffer-other-window behaves as expected:
> 
> --8<---------------cut here---------------start------------->8---
> (defun my-clone-indirect-buffer-other-window (newname display-flag &optional norecord)
>   "Like `clone-indirect-buffer' but display in another window."
>   (interactive
>    (progn
>      (if (get major-mode 'no-clone-indirect)
> 	 (error "Cannot indirectly clone a buffer in %s mode" mode-name))
>      (list (if current-prefix-arg
> 	       (read-buffer "Name of indirect buffer: " (current-buffer)))
> 	   t)))
>   (switch-to-buffer-other-window
>    (clone-indirect-buffer newname display-flag norecord)))
> --8<---------------cut here---------------end--------------->8---

Martin, any comments?

Adding also Stefan, in case he wants to comment.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: 29.3; clone-indirect-buffer-other-window is inconsistent with *-other-window commands
  2024-05-11  9:29 ` Eli Zaretskii
@ 2024-05-12  8:30   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-13  2:26   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 13+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-12  8:30 UTC (permalink / raw)
  To: Eli Zaretskii, Richard Sent, Stefan Monnier; +Cc: 70819

 >>    (switch-to-buffer-other-window
 >>     (clone-indirect-buffer newname display-flag norecord)))

If DISPLAY-FLAG is non-nil, 'clone-indirect-buffer' will by default try
to pop to a clone of the current buffer in the selected window via

       (pop-to-buffer buffer nil norecord))

What is 'switch-to-buffer-other-window' supposed to do then?

I suppose what you mean is

   (let ((buffer (clone-indirect-buffer newname nil norecord)))
     (when display-flag
       (switch-to-buffer-other-window buffer norecord))
     buffer)

martin





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: 29.3; clone-indirect-buffer-other-window is inconsistent with *-other-window commands
  2024-05-11  9:29 ` Eli Zaretskii
  2024-05-12  8:30   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-13  2:26   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-13  3:14     ` bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window Richard Sent
  1 sibling, 1 reply; 13+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-13  2:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: martin rudalics, Richard Sent, 70819

>> (defun my-clone-indirect-buffer-other-window (newname display-flag &optional norecord)
>>   "Like `clone-indirect-buffer' but display in another window."
>>   (interactive
>>    (progn
>>      (if (get major-mode 'no-clone-indirect)
>> 	 (error "Cannot indirectly clone a buffer in %s mode" mode-name))
>>      (list (if current-prefix-arg
>> 	       (read-buffer "Name of indirect buffer: " (current-buffer)))
>> 	   t)))
>>   (switch-to-buffer-other-window
>>    (clone-indirect-buffer newname display-flag norecord)))

`switch-to-buffer-other-window` sounds good, but we need to get rid of
`display-flag` (it makes no sense to call
`clone-indirect-buffer-other-window` with a nil value for
`display-flag` and `clone-indirect-buffer-other-window` should not call
`clone-indirect-buffer` with a non-nil `display-flag` because it wants
to be in charge of displaying the buffer).


        Stefan






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window
  2024-05-13  2:26   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-13  3:14     ` Richard Sent
  2024-05-13  5:22       ` Eli Zaretskii
  2024-05-13  8:05       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 13+ messages in thread
From: Richard Sent @ 2024-05-13  3:14 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: martin rudalics, Eli Zaretskii, 70819

Previously, depending on the settings in display-buffer-alist,
clone-indirect-buffer-other-window would display the cloned buffer in
the original window, behaving identically to clone-indirect-buffer with
a non-nil display-flag. This behavior was inconsistent with other-window
commands, which always used another window.

Now, clone-indirect-buffer-other-window uses
switch-to-buffer-other-window. This means it uses the same logic as
other-window commands like find-file-other-window and info-other-window
and its behavior is more consistent.

Because there is no reason for an other-window command to take a
display-flag argument, the argument was removed.

* lisp/simple.el: (clone-indirect-buffer-other-window): Use
switch-to-buffer-other-window and remove display-flag.
---
That makes sense to me, no complaints and things work as expected given
the change you suggest. Thank you all for taking the time to look into
this!

Hopefully this patch is sent successfully and makes things just a little
bit easier.

 lisp/simple.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index deab52c4201..0ab502c9f6b 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10649,7 +10649,7 @@ Returns the newly created indirect buffer."
     buffer))
 
 
-(defun clone-indirect-buffer-other-window (newname display-flag &optional norecord)
+(defun clone-indirect-buffer-other-window (newname &optional norecord)
   "Like `clone-indirect-buffer' but display in another window."
   (interactive
    (progn
@@ -10658,8 +10658,8 @@ Returns the newly created indirect buffer."
      (list (if current-prefix-arg
 	       (read-buffer "Name of indirect buffer: " (current-buffer)))
 	   t)))
-  (let ((pop-up-windows t))
-    (clone-indirect-buffer newname display-flag norecord)))
+  (switch-to-buffer-other-window
+   (clone-indirect-buffer newname nil norecord)))
 
 \f
 ;;; Handling of Backspace and Delete keys.
-- 
2.41.0






^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window
  2024-05-13  3:14     ` bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window Richard Sent
@ 2024-05-13  5:22       ` Eli Zaretskii
  2024-05-13  8:05       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2024-05-13  5:22 UTC (permalink / raw)
  To: Richard Sent; +Cc: rudalics, 70819, monnier

> From: Richard Sent <richard@freakingpenguin.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  martin rudalics <rudalics@gmx.at>,
>   70819@debbugs.gnu.org
> Date: Sun, 12 May 2024 23:14:19 -0400
> 
> Previously, depending on the settings in display-buffer-alist,
> clone-indirect-buffer-other-window would display the cloned buffer in
> the original window, behaving identically to clone-indirect-buffer with
> a non-nil display-flag. This behavior was inconsistent with other-window
> commands, which always used another window.
> 
> Now, clone-indirect-buffer-other-window uses
> switch-to-buffer-other-window. This means it uses the same logic as
> other-window commands like find-file-other-window and info-other-window
> and its behavior is more consistent.
> 
> Because there is no reason for an other-window command to take a
> display-flag argument, the argument was removed.

Thanks, but we cannot change an API of a public function.  We must
keep the argument, but not use it (and a NEWS entry should say that it
is now unused).





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window
  2024-05-13  3:14     ` bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window Richard Sent
  2024-05-13  5:22       ` Eli Zaretskii
@ 2024-05-13  8:05       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-25  7:49         ` Eli Zaretskii
  1 sibling, 1 reply; 13+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-13  8:05 UTC (permalink / raw)
  To: Richard Sent, Stefan Monnier; +Cc: Eli Zaretskii, 70819

 > -(defun clone-indirect-buffer-other-window (newname display-flag &optional norecord)
 > +(defun clone-indirect-buffer-other-window (newname &optional norecord)
 >     "Like `clone-indirect-buffer' but display in another window."
 >     (interactive
 >      (progn
 > @@ -10658,8 +10658,8 @@ Returns the newly created indirect buffer."
 >        (list (if current-prefix-arg
 >   	       (read-buffer "Name of indirect buffer: " (current-buffer)))
 >   	   t)))
 > -  (let ((pop-up-windows t))
 > -    (clone-indirect-buffer newname display-flag norecord)))
 > +  (switch-to-buffer-other-window
 > +   (clone-indirect-buffer newname nil norecord)))

I see two problems:

(1) 'clone-indirect-buffer-other-window' would still have to obey the
     DISPLAY_FLAG argument.  Some application may rely on the fact that
     'clone-indirect-buffer-other-window' doesn't display the buffer if
     it is nil.

(2) 'switch-to-buffer-other-window' does not get passed the NORECORD
     argument.

martin





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window
  2024-05-13  8:05       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-25  7:49         ` Eli Zaretskii
  2024-06-08 11:45           ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2024-05-25  7:49 UTC (permalink / raw)
  To: richard; +Cc: martin rudalics, 70819, monnier

Ping!  Richard, could you please augment your pat6ch according to
Martin's comments and resubmit?

> Date: Mon, 13 May 2024 10:05:52 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, 70819@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > -(defun clone-indirect-buffer-other-window (newname display-flag &optional norecord)
>  > +(defun clone-indirect-buffer-other-window (newname &optional norecord)
>  >     "Like `clone-indirect-buffer' but display in another window."
>  >     (interactive
>  >      (progn
>  > @@ -10658,8 +10658,8 @@ Returns the newly created indirect buffer."
>  >        (list (if current-prefix-arg
>  >   	       (read-buffer "Name of indirect buffer: " (current-buffer)))
>  >   	   t)))
>  > -  (let ((pop-up-windows t))
>  > -    (clone-indirect-buffer newname display-flag norecord)))
>  > +  (switch-to-buffer-other-window
>  > +   (clone-indirect-buffer newname nil norecord)))
> 
> I see two problems:
> 
> (1) 'clone-indirect-buffer-other-window' would still have to obey the
>      DISPLAY_FLAG argument.  Some application may rely on the fact that
>      'clone-indirect-buffer-other-window' doesn't display the buffer if
>      it is nil.
> 
> (2) 'switch-to-buffer-other-window' does not get passed the NORECORD
>      argument.
> 
> martin
> 





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH v2] Make clone-indirect-buffer-other-window use other window (bug#70819)
  2024-05-07 16:19 bug#70819: 29.3; clone-indirect-buffer-other-window is inconsistent with *-other-window commands Richard Sent
  2024-05-11  9:29 ` Eli Zaretskii
@ 2024-05-27 15:32 ` Richard Sent
  2024-05-28  8:04   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-08 12:18   ` Eli Zaretskii
  1 sibling, 2 replies; 13+ messages in thread
From: Richard Sent @ 2024-05-27 15:32 UTC (permalink / raw)
  To: 70819; +Cc: rudalics, eliz, Richard Sent

Previously, depending on the settings in display-buffer-alist,
clone-indirect-buffer-other-window would display the cloned buffer in
the original window, behaving identically to clone-indirect-buffer with
a non-nil display-flag. This behavior was inconsistent with other-window
commands which always used another window.

Now, clone-indirect-buffer-other-window uses
switch-to-buffer-other-window. This means it uses the same logic as
other-window commands like find-file-other-window and info-other-window.

display-flag was kept for API stability and functional compatibility
reasons.

* lisp/simple.el: (clone-indirect-buffer-other-window): Use
switch-to-buffer-other-window.
---
Updated according to feedback from Martin.

 lisp/simple.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index deab52c4201..cf206252c1e 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10658,8 +10658,10 @@ clone-indirect-buffer-other-window
      (list (if current-prefix-arg
 	       (read-buffer "Name of indirect buffer: " (current-buffer)))
 	   t)))
-  (let ((pop-up-windows t))
-    (clone-indirect-buffer newname display-flag norecord)))
+  ;; For compatibility, don't display the buffer if display-flag is nil.
+  (let ((buffer (clone-indirect-buffer newname nil norecord)))
+    (when display-flag
+      (switch-to-buffer-other-window buffer norecord))))
 
 \f
 ;;; Handling of Backspace and Delete keys.
-- 
2.41.0






^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH v2] Make clone-indirect-buffer-other-window use other window (bug#70819)
  2024-05-27 15:32 ` bug#70819: [PATCH v2] Make clone-indirect-buffer-other-window use other window (bug#70819) Richard Sent
@ 2024-05-28  8:04   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-08 12:18   ` Eli Zaretskii
  1 sibling, 0 replies; 13+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-28  8:04 UTC (permalink / raw)
  To: Richard Sent, 70819; +Cc: eliz

 > -  (let ((pop-up-windows t))
 > -    (clone-indirect-buffer newname display-flag norecord)))
 > +  ;; For compatibility, don't display the buffer if display-flag is nil.
 > +  (let ((buffer (clone-indirect-buffer newname nil norecord)))
 > +    (when display-flag
 > +      (switch-to-buffer-other-window buffer norecord))))

Looks good to me.

Thank you, martin





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window
  2024-05-25  7:49         ` Eli Zaretskii
@ 2024-06-08 11:45           ` Eli Zaretskii
  2024-06-08 13:29             ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2024-06-08 11:45 UTC (permalink / raw)
  To: richard, rudalics; +Cc: 70819, monnier

Ping! Ping! Richard, please augment your patch according to Martin's
comments, so we could install this.

> Cc: martin rudalics <rudalics@gmx.at>, 70819@debbugs.gnu.org,
>  monnier@iro.umontreal.ca
> Date: Sat, 25 May 2024 10:49:56 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Ping!  Richard, could you please augment your pat6ch according to
> Martin's comments and resubmit?
> 
> > Date: Mon, 13 May 2024 10:05:52 +0200
> > Cc: Eli Zaretskii <eliz@gnu.org>, 70819@debbugs.gnu.org
> > From: martin rudalics <rudalics@gmx.at>
> > 
> >  > -(defun clone-indirect-buffer-other-window (newname display-flag &optional norecord)
> >  > +(defun clone-indirect-buffer-other-window (newname &optional norecord)
> >  >     "Like `clone-indirect-buffer' but display in another window."
> >  >     (interactive
> >  >      (progn
> >  > @@ -10658,8 +10658,8 @@ Returns the newly created indirect buffer."
> >  >        (list (if current-prefix-arg
> >  >   	       (read-buffer "Name of indirect buffer: " (current-buffer)))
> >  >   	   t)))
> >  > -  (let ((pop-up-windows t))
> >  > -    (clone-indirect-buffer newname display-flag norecord)))
> >  > +  (switch-to-buffer-other-window
> >  > +   (clone-indirect-buffer newname nil norecord)))
> > 
> > I see two problems:
> > 
> > (1) 'clone-indirect-buffer-other-window' would still have to obey the
> >      DISPLAY_FLAG argument.  Some application may rely on the fact that
> >      'clone-indirect-buffer-other-window' doesn't display the buffer if
> >      it is nil.
> > 
> > (2) 'switch-to-buffer-other-window' does not get passed the NORECORD
> >      argument.
> > 
> > martin
> > 
> 
> 
> 
> 





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH v2] Make clone-indirect-buffer-other-window use other window (bug#70819)
  2024-05-27 15:32 ` bug#70819: [PATCH v2] Make clone-indirect-buffer-other-window use other window (bug#70819) Richard Sent
  2024-05-28  8:04   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-08 12:18   ` Eli Zaretskii
  1 sibling, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2024-06-08 12:18 UTC (permalink / raw)
  To: Richard Sent; +Cc: rudalics, 70819-done

> From: Richard Sent <richard@freakingpenguin.com>
> Cc: rudalics@gmx.at,
> 	eliz@gnu.org,
> 	Richard Sent <richard@freakingpenguin.com>
> Date: Mon, 27 May 2024 11:32:00 -0400
> 
> Previously, depending on the settings in display-buffer-alist,
> clone-indirect-buffer-other-window would display the cloned buffer in
> the original window, behaving identically to clone-indirect-buffer with
> a non-nil display-flag. This behavior was inconsistent with other-window
> commands which always used another window.
> 
> Now, clone-indirect-buffer-other-window uses
> switch-to-buffer-other-window. This means it uses the same logic as
> other-window commands like find-file-other-window and info-other-window.
> 
> display-flag was kept for API stability and functional compatibility
> reasons.
> 
> * lisp/simple.el: (clone-indirect-buffer-other-window): Use
> switch-to-buffer-other-window.

Thanks, installed on master, and closing the bug.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window
  2024-06-08 11:45           ` Eli Zaretskii
@ 2024-06-08 13:29             ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2024-06-08 13:29 UTC (permalink / raw)
  To: richard, rudalics; +Cc: 70819, monnier

> Cc: 70819@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Sat, 08 Jun 2024 14:45:57 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Ping! Ping! Richard, please augment your patch according to Martin's
> comments, so we could install this.

Sorry, you already did, and I've installed the changes.





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-06-08 13:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-07 16:19 bug#70819: 29.3; clone-indirect-buffer-other-window is inconsistent with *-other-window commands Richard Sent
2024-05-11  9:29 ` Eli Zaretskii
2024-05-12  8:30   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-13  2:26   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-13  3:14     ` bug#70819: [PATCH] Make clone-indirect-buffer-other-window use other window Richard Sent
2024-05-13  5:22       ` Eli Zaretskii
2024-05-13  8:05       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-25  7:49         ` Eli Zaretskii
2024-06-08 11:45           ` Eli Zaretskii
2024-06-08 13:29             ` Eli Zaretskii
2024-05-27 15:32 ` bug#70819: [PATCH v2] Make clone-indirect-buffer-other-window use other window (bug#70819) Richard Sent
2024-05-28  8:04   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-08 12:18   ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).