unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
@ 2015-03-26 18:25 Dima Kogan
  2015-03-26 18:41 ` Eli Zaretskii
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Dima Kogan @ 2015-03-26 18:25 UTC (permalink / raw)
  To: 20206

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

When looking at a patch in diff-mode, one can visit the sources being
patched with several functions, for instance (diff-goto-source). This
patch makes sure that the source buffer does not cover up the diff-mode
buffer in its window, but uses any other window instead.

This really is a continuation of bug 20034 (and 17675 and 19901). I feel
like the earlier display-buffer implementation had a nicer default in
this regard. Should we keep adding this patch wherever a user operation
on a buffer can cause a different buffer to be displayed, or should
display-buffer defaults be changed to make the "don't steal user's
focus" behavior the default?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-keep-diff-mode-s-window-visible-when-we-visit-source.patch --]
[-- Type: text/x-diff, Size: 2784 bytes --]

From fa88533777a500784cb5def1733aa182c6bd4135 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Thu, 26 Mar 2015 11:21:06 -0700
Subject: [PATCH] keep diff-mode's window visible when we visit sources from
 diff-mode

When looking at a patch in diff-mode, one can visit the sources being
patched with several functions, for instance (diff-goto-source).  This
patch makes sure that the source buffer does not cover up the
diff-mode buffer in its window, but uses any other window instead
---
 lisp/vc/diff-mode.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index a9614e9..15cdae9 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -1318,7 +1318,7 @@ See `after-change-functions' for the meaning of BEG, END and LEN."
   ;; Select a window that displays the current buffer so that point
   ;; movements are reflected in that window.  Otherwise, the user might
   ;; never see the hunk corresponding to the source she's jumping to.
-  (pop-to-buffer (current-buffer))
+  (pop-to-buffer (current-buffer) '(display-buffer-use-some-window (inhibit-same-window . t)))
   (if reset (goto-char (point-min)))
   (diff-hunk-next arg)
   (diff-goto-source))
@@ -1801,7 +1801,7 @@ With a prefix argument, REVERSE the hunk."
 	(delete-region (car pos) (cdr pos))
 	(insert (car new)))
       ;; Display BUF in a window
-      (set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
+      (set-window-point (display-buffer buf '(display-buffer-use-some-window (inhibit-same-window . t))) (+ (car pos) (cdr new)))
       (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil)
       (when diff-advance-after-apply-hunk
 	(diff-hunk-next))))))
@@ -1813,7 +1813,7 @@ With a prefix argument, try to REVERSE the hunk."
   (interactive "P")
   (pcase-let ((`(,buf ,line-offset ,pos ,src ,_dst ,switched)
                (diff-find-source-location nil reverse)))
-    (set-window-point (display-buffer buf) (+ (car pos) (cdr src)))
+    (set-window-point (display-buffer buf '(display-buffer-use-some-window (inhibit-same-window . t))) (+ (car pos) (cdr src)))
     (diff-hunk-status-msg line-offset (diff-xor reverse switched) t)))
 
 
@@ -1843,7 +1843,7 @@ then `diff-jump-to-old-file' is also set, for the next invocations."
   (let ((rev (not (save-excursion (beginning-of-line) (looking-at "[-<]")))))
     (pcase-let ((`(,buf ,line-offset ,pos ,src ,_dst ,switched)
                  (diff-find-source-location other-file rev)))
-      (pop-to-buffer buf)
+      (pop-to-buffer buf '(display-buffer-use-some-window (inhibit-same-window . t)))
       (goto-char (+ (car pos) (cdr src)))
       (diff-hunk-status-msg line-offset (diff-xor rev switched) t))))
 
-- 
2.1.4


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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-26 18:25 bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode Dima Kogan
@ 2015-03-26 18:41 ` Eli Zaretskii
  2015-03-26 18:58 ` martin rudalics
  2019-06-25 17:33 ` Lars Ingebrigtsen
  2 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2015-03-26 18:41 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 20206

> From: Dima Kogan <dima@secretsauce.net>
> Date: Thu, 26 Mar 2015 11:25:27 -0700
> 
> When looking at a patch in diff-mode, one can visit the sources being
> patched with several functions, for instance (diff-goto-source). This
> patch makes sure that the source buffer does not cover up the diff-mode
> buffer in its window, but uses any other window instead.

IMO, this should be controlled by a user option, since we can never be
sure there isn't someone who likes the current behavior.  Not having
an option will not allow them to get the old behavior back, which IMO
is not a good idea.

Thanks.





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-26 18:25 bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode Dima Kogan
  2015-03-26 18:41 ` Eli Zaretskii
@ 2015-03-26 18:58 ` martin rudalics
  2015-03-28  6:01   ` Dima Kogan
  2015-03-28 21:53   ` Dima Kogan
  2019-06-25 17:33 ` Lars Ingebrigtsen
  2 siblings, 2 replies; 17+ messages in thread
From: martin rudalics @ 2015-03-26 18:58 UTC (permalink / raw)
  To: Dima Kogan, 20206

 > -  (pop-to-buffer (current-buffer))

Something must be wrong here.  According to its doc-string
`pop-to-buffer' should

"Select buffer BUFFER in some window, preferably a different one."

But I nowhere see that it tries to enforce that.  Am I blind?  And
`display-buffer' should not use the selected window either, after all
it's likely the most recently used one.  Could you please debug this in
order to explain why the selected window gets used in all these cases?

 > +  (pop-to-buffer (current-buffer) '(display-buffer-use-some-window (inhibit-same-window . t)))

In any case `display-buffer-use-some-window' as sole action doesn't look
right.  It should be preceded by `display-buffer-reuse-window' and
`display-buffer-pop-up-window'.  And `pop-to-buffer' should do that
already.

Thanks, martin





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-26 18:58 ` martin rudalics
@ 2015-03-28  6:01   ` Dima Kogan
  2015-03-28  9:58     ` martin rudalics
  2015-03-28 21:53   ` Dima Kogan
  1 sibling, 1 reply; 17+ messages in thread
From: Dima Kogan @ 2015-03-28  6:01 UTC (permalink / raw)
  To: martin rudalics; +Cc: 20206


martin rudalics <rudalics@gmx.at> writes:

>  > -  (pop-to-buffer (current-buffer))
>
> Something must be wrong here.  According to its doc-string
> `pop-to-buffer' should
>
> "Select buffer BUFFER in some window, preferably a different one."
>
> But I nowhere see that it tries to enforce that.  Am I blind?

I don't see it either. (pop-to-buffer buf) simply does (display-buffer
buf).


> And `display-buffer' should not use the selected window either, after
> all it's likely the most recently used one. Could you please debug
> this in order to explain why the selected window gets used in all
> these cases?

I think it uses the most recent window a buffer was displayed in. If
this most-recenty-used window now shows a different buffer, then this
new buffer is covered up. Example:

emacs24 -Q --eval '(progn
  (split-window-horizontally)     # split into left,right; *scratch* in each
  (other-window 1)                # focus the right window (*scratch*)
  (switch-to-buffer "asdf")       # right window now has buffer "asdf"
  (other-window 1)                # focus the left window (*scratch*)
  (switch-to-buffer "*Messages*") # left window now has buffer *Messages*
  (switch-to-buffer "*scratch*")  # left window back to buffer *scratch*
  (display-buffer "*Messages*")   # display *Messages* somewhere
 )'

Running the above covers up the focused *scratch* buffer in the left
window. Running it in emacs23 instead does NOT cover it up, which is the
behavior that feels more right to me. The recent patches try to get back
to this state.

If we do not do the

 (switch-to-buffer "*Messages*") (switch-to-buffer "*scratch*")

dance then the *scratch* buffer isn't covered up even in emacs24.


>  > +  (pop-to-buffer (current-buffer) '(display-buffer-use-some-window (inhibit-same-window . t)))
>
> In any case `display-buffer-use-some-window' as sole action doesn't look
> right.  It should be preceded by `display-buffer-reuse-window' and
> `display-buffer-pop-up-window'.  And `pop-to-buffer' should do that
> already.

OK, but if the larger question is now on the table, (about what should
(pop-to-buffer) and (display-buffer) do by default), we should probably
answer that first.

dima





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-28  6:01   ` Dima Kogan
@ 2015-03-28  9:58     ` martin rudalics
  2015-03-28 21:44       ` Dima Kogan
  0 siblings, 1 reply; 17+ messages in thread
From: martin rudalics @ 2015-03-28  9:58 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 20206

 >> And `display-buffer' should not use the selected window either, after
 >> all it's likely the most recently used one. Could you please debug
 >> this in order to explain why the selected window gets used in all
 >> these cases?
 >
 > I think it uses the most recent window a buffer was displayed in. If
 > this most-recenty-used window now shows a different buffer, then this
 > new buffer is covered up. Example:
 >
 > emacs24 -Q --eval '(progn
 >    (split-window-horizontally)     # split into left,right; *scratch* in each
 >    (other-window 1)                # focus the right window (*scratch*)
 >    (switch-to-buffer "asdf")       # right window now has buffer "asdf"
 >    (other-window 1)                # focus the left window (*scratch*)
 >    (switch-to-buffer "*Messages*") # left window now has buffer *Messages*
 >    (switch-to-buffer "*scratch*")  # left window back to buffer *scratch*
 >    (display-buffer "*Messages*")   # display *Messages* somewhere
 >   )'
 >
 > Running the above covers up the focused *scratch* buffer in the left
 > window. Running it in emacs23 instead does NOT cover it up, which is the
 > behavior that feels more right to me. The recent patches try to get back
 > to this state.
 >
 > If we do not do the
 >
 >   (switch-to-buffer "*Messages*") (switch-to-buffer "*scratch*")
 >
 > dance then the *scratch* buffer isn't covered up even in emacs24.

I see.  This is due to `display-buffer-in-previous-window' which covers
such idiosyncrasies like *Backtrace* alternately appearing in one and
the other window (because the least recently used window alternates
continuously).  Your `switch-to-buffer' calls imply that you do want to
see both *Messages* and *scratch* preferably in the left window and
`display-buffer' complies.

You can switch this off by customizing `display-buffer-base-action'
accordingly.

 > OK, but if the larger question is now on the table, (about what should
 > (pop-to-buffer) and (display-buffer) do by default), we should probably
 > answer that first.

Can you tell me whether and how `display-buffer-in-previous-window' is
used in the `diff-mode' scenario (I use a highly customized `ediff-mode'
so I can't easily check that myself).  Does it use `switch-to-buffer'?

martin





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-28  9:58     ` martin rudalics
@ 2015-03-28 21:44       ` Dima Kogan
  2015-03-29 18:00         ` martin rudalics
  0 siblings, 1 reply; 17+ messages in thread
From: Dima Kogan @ 2015-03-28 21:44 UTC (permalink / raw)
  To: martin rudalics; +Cc: 20206

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

martin rudalics <rudalics@gmx.at> writes:

> You can switch this off by customizing `display-buffer-base-action'
> accordingly.
>
>  > OK, but if the larger question is now on the table, (about what should
>  > (pop-to-buffer) and (display-buffer) do by default), we should probably
>  > answer that first.
>
> Can you tell me whether and how `display-buffer-in-previous-window' is
> used in the `diff-mode' scenario (I use a highly customized `ediff-mode'
> so I can't easily check that myself).  Does it use `switch-to-buffer'?

The diff-mode scenario is the same as the other example. M-enter invokes
(diff-goto-source), which invokes (pop-to-buffer), which invokes
(display-buffer). Here's the previous example, but using diff-mode
instead. Assuming "dat" and "dat.patch" exist in the current directory
(attached):


emacs -Q --eval '(progn

  (find-file "dat")
  (find-file "dat.patch")        # open both files; one window/frame
  (split-window-horizontally)    # two windows; both show "dat.patch"
  (other-window 1)               # switch to right window
  (switch-to-buffer "asdf")      # switch to dummy "asdf" buffer
  (other-window 1)               # left window active with "dat.patch",
                                 # right shows "asdf"

  (switch-to-buffer "dat")
  (switch-to-buffer "dat.patch") # left window: show "dat" and then back
                                 # to "dat.patch"

  (diff-goto-source))            # navigate to the diff source


In emacs23 the "dat" buffer is shown in the window OTHER than the one
showing the diff. In emacs24, the diff window is covered up.



[-- Attachment #2: dat --]
[-- Type: application/octet-stream, Size: 10 bytes --]

1
2
3
4
5

[-- Attachment #3: dat.patch --]
[-- Type: application/octet-stream, Size: 119 bytes --]

--- dat	2015-03-28 13:42:37.893813464 -0700
+++ dat	2015-03-28 13:42:20.729728352 -0700
@@ -1,5 +1,4 @@
 1
 2
-3
 4
 5

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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-26 18:58 ` martin rudalics
  2015-03-28  6:01   ` Dima Kogan
@ 2015-03-28 21:53   ` Dima Kogan
  2015-03-29 18:01     ` martin rudalics
  1 sibling, 1 reply; 17+ messages in thread
From: Dima Kogan @ 2015-03-28 21:53 UTC (permalink / raw)
  To: martin rudalics; +Cc: 20206


martin rudalics <rudalics@gmx.at> writes:

>  > +  (pop-to-buffer (current-buffer) '(display-buffer-use-some-window (inhibit-same-window . t)))
>
> In any case `display-buffer-use-some-window' as sole action doesn't look
> right.  It should be preceded by `display-buffer-reuse-window' and
> `display-buffer-pop-up-window'.

To be clear, this action came from two desires:

1. The (inhibit-same-window . t) is there to get the
dont-cover-up-control-buffer behavior

2. The display-buffer-pop-up-window action is there to handle bug 19901.
There the user already has two windows, and by default a third is
created. It's less obvious to me which is the "right" behavior here, but
the earlier behavior of reusing the window seems reasonable.





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-28 21:44       ` Dima Kogan
@ 2015-03-29 18:00         ` martin rudalics
  2015-03-29 20:06           ` Dima Kogan
  0 siblings, 1 reply; 17+ messages in thread
From: martin rudalics @ 2015-03-29 18:00 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 20206

 >    (find-file "dat")
 >    (find-file "dat.patch")        # open both files; one window/frame

You here ...

 >    (split-window-horizontally)    # two windows; both show "dat.patch"
 >    (other-window 1)               # switch to right window
 >    (switch-to-buffer "asdf")      # switch to dummy "asdf" buffer
 >    (other-window 1)               # left window active with "dat.patch",
 >                                   # right shows "asdf"
 >
 >    (switch-to-buffer "dat")
 >    (switch-to-buffer "dat.patch") # left window: show "dat" and then back
 >                                   # to "dat.patch"

... and here tell Emacs that you very much like having dat and dat.patch
share the same window.  OTOH, the split-window-horizontally,
other-window, switch-to-buffer "asdf" sequence indicates that you rather
want the other window keep showing asdf.  So `display-buffer' will have
a hard time concluding here ...

 >    (diff-goto-source))            # navigate to the diff source

... that you want to show dat in the window formerly occupied by asdf.

 > In emacs23 the "dat" buffer is shown in the window OTHER than the one
 > showing the diff. In emacs24, the diff window is covered up.

I see.  I don't yet have a solution for this.

martin





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-28 21:53   ` Dima Kogan
@ 2015-03-29 18:01     ` martin rudalics
  2015-03-29 19:48       ` Dima Kogan
  0 siblings, 1 reply; 17+ messages in thread
From: martin rudalics @ 2015-03-29 18:01 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 20206

 >>   > +  (pop-to-buffer (current-buffer) '(display-buffer-use-some-window (inhibit-same-window . t)))
 >>
 >> In any case `display-buffer-use-some-window' as sole action doesn't look
 >> right.  It should be preceded by `display-buffer-reuse-window' and
 >> `display-buffer-pop-up-window'.
 >
 > To be clear, this action came from two desires:
 >
 > 1. The (inhibit-same-window . t) is there to get the
 > dont-cover-up-control-buffer behavior

OK.

 > 2. The display-buffer-pop-up-window action is there to handle bug 19901.
 > There the user already has two windows, and by default a third is
 > created. It's less obvious to me which is the "right" behavior here, but
 > the earlier behavior of reusing the window seems reasonable.

Agreed.  But if you have a two windows frame and your settings for
popping up a third window would normally allow that, your solution would
nevertheless try to use another window in that case.  Wouldn't it be
sufficient to just use '((inhibit-same-window . t))?

martin





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-29 18:01     ` martin rudalics
@ 2015-03-29 19:48       ` Dima Kogan
  2015-03-30  8:37         ` martin rudalics
  0 siblings, 1 reply; 17+ messages in thread
From: Dima Kogan @ 2015-03-29 19:48 UTC (permalink / raw)
  To: martin rudalics; +Cc: 20206


martin rudalics <rudalics@gmx.at> writes:

>  > 2. The display-buffer-pop-up-window action is there to handle bug 19901.
>  > There the user already has two windows, and by default a third is
>  > created. It's less obvious to me which is the "right" behavior here, but
>  > the earlier behavior of reusing the window seems reasonable.
>
> Agreed.  But if you have a two windows frame and your settings for
> popping up a third window would normally allow that, your solution would
> nevertheless try to use another window in that case.  Wouldn't it be
> sufficient to just use '((inhibit-same-window . t))?

I'm not sure what you mean: '((inhibit-same-window . t)) is not a valid
as an action argument to display-buffer, and emacs barfs if I try to use
it as such. I'm not at all married to using display-buffer-pop-up-window
here. If you have a better idea for solving 19901 (or if you think
there's nothing to fix), then I'm fine with it.

Another wrinkle is that the behavior reported in 19901 as a bug is only
observable with particular geometries of the emacs X11 window. On my
machine, 19901 is reproducible if emacs takes up my whole screen
(1440x1050), but not if it takes only half (720x1050). Presumably emacs
thinks that splitting into a 3rd window will create new windows that are
too narrow. That is all to say that maybe 19901 is not a bug.





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-29 18:00         ` martin rudalics
@ 2015-03-29 20:06           ` Dima Kogan
  2015-03-30  8:37             ` martin rudalics
  0 siblings, 1 reply; 17+ messages in thread
From: Dima Kogan @ 2015-03-29 20:06 UTC (permalink / raw)
  To: martin rudalics; +Cc: 20206

martin rudalics <rudalics@gmx.at> writes:

>  >    (find-file "dat")
>  >    (find-file "dat.patch")        # open both files; one window/frame
>
> You here ...
>
>  >    (split-window-horizontally)    # two windows; both show "dat.patch"
>  >    (other-window 1)               # switch to right window
>  >    (switch-to-buffer "asdf")      # switch to dummy "asdf" buffer
>  >    (other-window 1)               # left window active with "dat.patch",
>  >                                   # right shows "asdf"
>  >
>  >    (switch-to-buffer "dat")
>  >    (switch-to-buffer "dat.patch") # left window: show "dat" and then back
>  >                                   # to "dat.patch"
>
> ... and here tell Emacs that you very much like having dat and dat.patch
> share the same window.  OTOH, the split-window-horizontally,
> other-window, switch-to-buffer "asdf" sequence indicates that you rather
> want the other window keep showing asdf.  So `display-buffer' will have
> a hard time concluding here ...
>
>  >    (diff-goto-source))            # navigate to the diff source
>
> ... that you want to show dat in the window formerly occupied by asdf.

Well sure. This was a contrived example specifically constructed to show
this behavior. The original bug report came from seeing this during
normal use of gud, so I still think the complaint is valid.


>  > In emacs23 the "dat" buffer is shown in the window OTHER than the one
>  > showing the diff. In emacs24, the diff window is covered up.
>
> I see.  I don't yet have a solution for this.

I think this is only a question of defaults. Specifically, any time we
(pop-to-buffer) or (display-buffer) in response to an interactive user
action, we should default to not covering up the user's buffer. This
would apply to use cases like gud, diff, cscope and so on. I don't know
enough about the development of this to know if it makes sense to change
the defaults only in the interactive case or in general. Do you see
reasons to keep the default as it is?





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-29 19:48       ` Dima Kogan
@ 2015-03-30  8:37         ` martin rudalics
  0 siblings, 0 replies; 17+ messages in thread
From: martin rudalics @ 2015-03-30  8:37 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 20206

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

 > I'm not sure what you mean: '((inhibit-same-window . t)) is not a valid
 > as an action argument to display-buffer,

Sorry, I meant '(nil (inhibit-same-window . t)).

 > and emacs barfs if I try to use
 > it as such. I'm not at all married to using display-buffer-pop-up-window
 > here. If you have a better idea for solving 19901 (or if you think
 > there's nothing to fix), then I'm fine with it.

I'm afraid we are miscommunicating or I am misssing something.  You
propose the following change:

-  (pop-to-buffer (current-buffer))
+  (pop-to-buffer (current-buffer) '(display-buffer-use-some-window (inhibit-same-window . t)))

I think this precludes the following scenario: Consider a frame that
contains two windows.  The user has set `split-height-threshold' and/or
`split-width-threshold' to very small values which easily allow further
splitting.  In this case, a `pop-to-buffer' call for a buffer that
doesn't appear on the frame yet will usually try to split one of the two
windows and show the buffer in a third one.  Agreed?

Now with your proposed change `pop-to-buffer' will always use the "other
window" for displaying the current buffer, thus overriding the user's
customizations of `split-height-threshold' and `split-width-threshold'.
The reason is that `display-buffer-pop-up-window' will not be called
because `display-buffer-use-some-window' is always called first.  Do you
agree?  And can we agree that such behavior would be wrong?

Also, a user who wants to show the buffer in a separate frame via say

(setq pop-up-frames t)

will be disappointed as well.  With a two windows frame he'll never get
a new frame.  Agreed?

Maybe

(pop-to-buffer (current-buffer) t)

is already what we want here because we know that the current buffer is
not shown in the selected window.  Otherwise we could try a patch like
the one I attached.  Then we could do ...

(pop-to-buffer (current-buffer) '(nil (previous-window)))

... if I got it right this time.  WDYT?

 > Another wrinkle is that the behavior reported in 19901 as a bug is only
 > observable with particular geometries of the emacs X11 window. On my
 > machine, 19901 is reproducible if emacs takes up my whole screen
 > (1440x1050), but not if it takes only half (720x1050). Presumably emacs
 > thinks that splitting into a 3rd window will create new windows that are
 > too narrow. That is all to say that maybe 19901 is not a bug.

I have no problems with Bug#19901.  Let's care only about Bug#20206
here.

martin

[-- Attachment #2: window.el.diff --]
[-- Type: text/plain, Size: 4261 bytes --]

--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6371,6 +6371,10 @@ Recognized alist entries include:
                       window that already displays the buffer.
                       See `display-buffer-reuse-window'.
 
+ `previous-window' -- A window `display-buffer-in-previous-window'
+                      should use or nil to not try finding such a
+                      window.
+
  `pop-up-frame-parameters' -- Value specifies an alist of frame
                               parameters to give a new frame, if
                               one is created.
@@ -6678,43 +6682,47 @@ selected frame if `display-buffer-reuse-frames' and
 `pop-up-frames' are both nil; search all frames on the current
 terminal if either of those variables is non-nil.
 
-If ALIST has a `previous-window' entry, the window specified by
-that entry will override any other window found by the methods
-above, even if that window never showed BUFFER before."
-  (let* ((alist-entry (assq 'reusable-frames alist))
-	 (inhibit-same-window
-	  (cdr (assq 'inhibit-same-window alist)))
-	 (frames (cond
-		  (alist-entry (cdr alist-entry))
-		  ((if (eq pop-up-frames 'graphic-only)
-		       (display-graphic-p)
-		     pop-up-frames)
-		   0)
-		  (display-buffer-reuse-frames 0)
-		  (t (last-nonminibuffer-frame))))
-	 best-window second-best-window window)
-    ;; Scan windows whether they have shown the buffer recently.
-    (catch 'best
-      (dolist (window (window-list-1 (frame-first-window) 'nomini frames))
-	(when (and (assq buffer (window-prev-buffers window))
-		   (not (window-dedicated-p window)))
-	  (if (eq window (selected-window))
-	      (unless inhibit-same-window
-		(setq second-best-window window))
-	    (setq best-window window)
-	    (throw 'best t)))))
-    ;; When ALIST has a `previous-window' entry, that entry may override
-    ;; anything we found so far.
-    (when (and (setq window (cdr (assq 'previous-window alist)))
-	       (window-live-p window)
-	       (not (window-dedicated-p window)))
-      (if (eq window (selected-window))
-	  (unless inhibit-same-window
-	    (setq second-best-window window))
-	(setq best-window window)))
-    ;; Return best or second best window found.
-    (when (setq window (or best-window second-best-window))
-      (window--display-buffer buffer window 'reuse alist))))
+If ALIST has a `previous-window' entry, the window specified in
+the cdr of that entry will override any other window found by the
+methods above, even if that window never showed BUFFER before.
+If the cdr of that entry is nil, do not try to find a window that
+previously showed BUFFER."
+  (unless (let ((previous-window (assq 'previous-window alist)))
+	    (and previous-window (not (cdr previous-window))))
+    (let* ((alist-entry (assq 'reusable-frames alist))
+	   (inhibit-same-window
+	    (cdr (assq 'inhibit-same-window alist)))
+	   (frames (cond
+		    (alist-entry (cdr alist-entry))
+		    ((if (eq pop-up-frames 'graphic-only)
+			 (display-graphic-p)
+		       pop-up-frames)
+		     0)
+		    (display-buffer-reuse-frames 0)
+		    (t (last-nonminibuffer-frame))))
+	   best-window second-best-window window)
+      ;; Scan windows whether they have shown the buffer recently.
+      (catch 'best
+	(dolist (window (window-list-1 (frame-first-window) 'nomini frames))
+	  (when (and (assq buffer (window-prev-buffers window))
+		     (not (window-dedicated-p window)))
+	    (if (eq window (selected-window))
+		(unless inhibit-same-window
+		  (setq second-best-window window))
+	      (setq best-window window)
+	      (throw 'best t)))))
+      ;; When ALIST has a `previous-window' entry, that entry may override
+      ;; anything we found so far.
+      (when (and (setq window (cdr (assq 'previous-window alist)))
+		 (window-live-p window)
+		 (not (window-dedicated-p window)))
+	(if (eq window (selected-window))
+	    (unless inhibit-same-window
+	      (setq second-best-window window))
+	  (setq best-window window)))
+      ;; Return best or second best window found.
+      (when (setq window (or best-window second-best-window))
+	(window--display-buffer buffer window 'reuse alist)))))
 
 (defun display-buffer-use-some-window (buffer alist)
   "Display BUFFER in an existing window.


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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-29 20:06           ` Dima Kogan
@ 2015-03-30  8:37             ` martin rudalics
  0 siblings, 0 replies; 17+ messages in thread
From: martin rudalics @ 2015-03-30  8:37 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 20206

 > Well sure. This was a contrived example specifically constructed to show
 > this behavior. The original bug report came from seeing this during
 > normal use of gud, so I still think the complaint is valid.

OK.  I believe you.

 > I think this is only a question of defaults. Specifically, any time we
 > (pop-to-buffer) or (display-buffer) in response to an interactive user
 > action, we should default to not covering up the user's buffer. This
 > would apply to use cases like gud, diff, cscope and so on. I don't know
 > enough about the development of this to know if it makes sense to change
 > the defaults only in the interactive case or in general. Do you see
 > reasons to keep the default as it is?

The exclusion would be one and one only: namely that the buffer these
functions are supposed to show has appeared in the selected window
before.  In general a user might want to see *Help* or *info* at a
location where she has seen it before and not literally jump through the
available windows of the frame.  Maybe this misfires in the case at hand.

martin





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
       [not found] <552BB316.1000204@gmx.at>
@ 2015-04-13 14:34 ` Eli Zaretskii
  2015-04-14 15:51   ` martin rudalics
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2015-04-13 14:34 UTC (permalink / raw)
  To: martin rudalics; +Cc: mrsam, 20206

> Date: Mon, 13 Apr 2015 14:14:14 +0200
> From: martin rudalics <rudalics@gmx.at>
> Cc: help-gnu-emacs@gnu.org
> 
>  > The change in behavior is when there are already two buffers being shown, with different files. So, with two files, a and b:
>  >
>  >
>  > $ emacs -Q a b
>  >
>  > I get just "b" displayed.
>  >
>  > ^X^B^Xo, cursor down, Enter.
>  >
>  > I get the "b" file in the top window, "a" file in the bottom window, with the cursor in the "a" file's window.
>  >
>  >
>  > ^X^B
>  >
>  > The buffer window now replaces the "a" file, where the cursor was, and the cursor now winds up in the buffer list window. That's the different behavior than what I'm used to.
>  >
>  >
>  > Now, if instead of doing ^X^B at this point, I press ^Xo, moving the cursor back to the "b" file's window, then ^X^B still opens the list buffer in the "a" file's window (also).
>  >
>  >
>  > So, maybe the change in behavior is that list-buffers' window is "sticky", and it tries to open the buffer window in the same window it was previously shown in, apparently. If that window is showing another file buffer, even if the cursor is still in that file buffer the buffer list window still gets opened there, replacing the buffer the cursor was in.
>  >
>  >
>  > So, depending on window history, ^X^B ends up either opening the buffer list window in some other window, than the one the cursor is currently in, or the same window where the cursor is. I'm pretty sure that in earlier versions of emacs, ^X^B never opened the list buffer in the same window the cursor was, at the time the list-buffers command was executed.
>  >
>  >
>  > And that's where my muscle memory is failing me now. I'm used to having multiple windows open; and with the cursor in one of them, ^X^B opening the buffer in some other window, and then typing ^Xo to jump into the buffer list window. Now, depending on where the buffer-list window was previously shown, it will now open in the same window with the cursor, and I realize belatedly, after I already jumped somewhere else with ^Xo.
> 
> Your explanation is correct.  The behavior is intended as a feature but
> inherently breaks the behavior of `pop-to-buffer', especially when used
> interactively.  I intend to fix this soonish for Emacs 25 and also post
> a simple workaround you can use for Emacs 24.4.

I think the following piece of magic will do what Sam wanted:

  (setq display-buffer-alist
        '(("\\*Buffer List\\*" .
           (display-buffer-pop-up-window '((inhibit-same-window . t))))))

If this is desired for all buffers, the regexp should be changed to
something that matches any buffer name.





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-04-13 14:34 ` Eli Zaretskii
@ 2015-04-14 15:51   ` martin rudalics
  0 siblings, 0 replies; 17+ messages in thread
From: martin rudalics @ 2015-04-14 15:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mrsam, 20206

 > I couldn't find an obvious way to contribute to the thread.

Please try a "wide reply", "reply to list" or "reply to all" to this
mail.

 > Anyway, I tried adding Eli's suggestions to ~/.emacs; this didn't seem
 > to make any difference, even if I temporarily remove everything else
 > from there.

What Eli meant was more or less

(setq display-buffer-alist '(("\\*Buffer List\\*" nil (inhibit-same-window . t))))

But this is still problematic because it will disallow reusing a window
that already shows the buffer list.  Even if you explicitly ask for it
as in

(setq display-buffer-alist
       '(("\\*Buffer List\\*"
       display-buffer-reuse-window (inhibit-same-window . t))))

it won't work when you do C-x C-b in the selected window and that window
shows the buffer list already.  You might call it a feature but when
`display-buffer' is used programmatically, popping up or using another
window to show a buffer already shown elsewhere is usually considered a
nuisance.

So something better is needed here.

martin





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2015-03-26 18:25 bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode Dima Kogan
  2015-03-26 18:41 ` Eli Zaretskii
  2015-03-26 18:58 ` martin rudalics
@ 2019-06-25 17:33 ` Lars Ingebrigtsen
  2019-06-25 18:10   ` Dima Kogan
  2 siblings, 1 reply; 17+ messages in thread
From: Lars Ingebrigtsen @ 2019-06-25 17:33 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 20206

Dima Kogan <dima@secretsauce.net> writes:

> When looking at a patch in diff-mode, one can visit the sources being
> patched with several functions, for instance (diff-goto-source). This
> patch makes sure that the source buffer does not cover up the diff-mode
> buffer in its window, but uses any other window instead.

This is what happens to me on the Emacs trunk -- has this been fixed in
the meantime?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode
  2019-06-25 17:33 ` Lars Ingebrigtsen
@ 2019-06-25 18:10   ` Dima Kogan
  0 siblings, 0 replies; 17+ messages in thread
From: Dima Kogan @ 2019-06-25 18:10 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 20206

Lars Ingebrigtsen <larsi@gnus.org> writes:

> This is what happens to me on the Emacs trunk -- has this been fixed
> in the meantime?

Oof, this was a long time ago. I want to say that I still see this now,
but intermittently. Probably closing this issue make sense, and I can
open a new one if I want to dig into this again.

Thanks.





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

end of thread, other threads:[~2019-06-25 18:10 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-26 18:25 bug#20206: 25.0.50; [PATCH] keep diff-mode's window visible when we visit sources from diff-mode Dima Kogan
2015-03-26 18:41 ` Eli Zaretskii
2015-03-26 18:58 ` martin rudalics
2015-03-28  6:01   ` Dima Kogan
2015-03-28  9:58     ` martin rudalics
2015-03-28 21:44       ` Dima Kogan
2015-03-29 18:00         ` martin rudalics
2015-03-29 20:06           ` Dima Kogan
2015-03-30  8:37             ` martin rudalics
2015-03-28 21:53   ` Dima Kogan
2015-03-29 18:01     ` martin rudalics
2015-03-29 19:48       ` Dima Kogan
2015-03-30  8:37         ` martin rudalics
2019-06-25 17:33 ` Lars Ingebrigtsen
2019-06-25 18:10   ` Dima Kogan
     [not found] <552BB316.1000204@gmx.at>
2015-04-13 14:34 ` Eli Zaretskii
2015-04-14 15:51   ` martin rudalics

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