unexpected behavior based on the value of "display-buffer-alist"

[brandelune]

The code that is proposed here:
https://github.com/nex3/perspective-el#some-musings-on-emacs-window-layouts
to stabilize emacs Windows behavior severely conflicts with magit's basic behavior.

(setq display-buffer-alist
'((".*" (display-buffer-reuse-window display-buffer-same-window))))

The issue I registered there involves 2 frames:
nex3/perspective-el#129

But even with one frame, magit's behavior is unsatisfactory.

With only one frame, here is the behavior:

c c on staged contents → should display the commit message interface
instead, it reopens the diff display and then, a new c is required to access the commit message interface.

With 2 frames, the same c c kills the frame in which it was called and calls a diff on the front buffer of the other frame.

[tarsius]

(setq magit-display-buffer-function 'magit-display-buffer-same-window-except-diff-v1)

might be all you need.

[brandelune]

Thank you. That was it. Shouldn't it be default behavior ?

[tarsius]

No, but we maybe we should do the same as dd83d2d for magit-display-buffer-traditional as well. On the other hand we have already decided against that, from the commit message:

Note that this commit deliberately does not change
magit-display-buffer-traditional, since that is the default for
magit-display-buffer-function, and changing its behavior might
annoy people who have display-buffer-base-action customized but
rely on magit defaults. In the case of
magit-display-buffer-same-window-except-diff-v1, we can be fairly
certain that its users are either not affected, or would see this as
a bugfix.

I think what you are doing falls into the category of drastic modifications that will inevitably lead to you having to adjust some other things. I doubt Magit is the only package that is negatively affected by this customization.

[brandelune]

Ok, thank you very much for the explanations.

[gcv]

@tarsius: I'm here because @brandelune used the display-buffer-alist setting above at my suggestion (https://github.com/nex3/perspective-el#some-musings-on-emacs-window-layouts), and has drawn my attention to its incompatibility with recent changes in Magit. I took the opportunity to re-read The Zen of Buffer Display.

Given that Magit now wants to display the diffs along with commit messages, and breaks under what I consider a necessary customization (and frankly, not at all drastic — especially considering that Magit has worked flawlessly with my settings for a decade, and I have not seen any other packages suffer), this seems like an appropriate time to use display-buffer-overriding-action. A little grepping suggests that magit-diff.el already uses it. And while the manual advises caution, given the critical nature of precise window control in this case, it seems like the right place to use it.

[tarsius]

I still consider it a radical customization to disallow packages from creating new windows by default. But lets agree to disagree. I just want to point out that Magit did not break due to this setting. It displayed the commit message in the selected window and then immediately displays the diff in the same window. With this setting you have to expect this sort of thing to happen in some contexts and learn how to quickly recover. In this case that can be done by pressing q to close the diff buffer.

I have adjusted magit-commit-diff and transient-display-buffer-action accordingly.

[gcv]

Perfect, thank you!

[tarsius]

Doing the "perfect" thing never comes without a price: https://www.reddit.com/r/emacs/comments/hllud/how_can_one_configure_magit_to_not_open_a_new/. I don't think I can work up the motivation to deal with that right now.

[acowley]

Yes, this change was certainly unwelcome for me. I never use more than one emacs frame, but this change leads to the creation of a new frame when committing, and that frame is not closed when finalizing or abandoning the commit. I use a tiling window manager on my desktop, the new frame shuffles things around in an unpleasant way, and then I need to use window manager keys rather than emacs keys to clean up the extra frame.

I am fine with reverting the patch that did this for myself, but I'm also willing to put some effort into coming up with a better solution. Would the addition of yet another configuration option to skip over the problematic line be okay, or is there still hope of finding something automatic that makes everyone happy?

[tarsius]

Yes, this change was certainly unwelcome for me

Just to be clear: the observed behavior was in no way intended. There was an edge-case that didn't work as intended. We addressed that. That fix then broke something else. Creating a new frame for this is awful.

I don't quite understand why such breaking changes are assumed to be intentional, when really they are obvious regressions.

I am fine with reverting the patch that did this for myself, but I'm also willing to put some effort into coming up with a better solution.

Wonderful. We don't want (1, see above) the diff buffer to "shadow" the git-commit buffer and (2) we don't want a second frame to pop up either. When the existing frame is small or the values of the "don't split frame if it is this small" variables are too small, then that is hard. The fix probably involves overriding these variables.

So we want to tell Emacs that we want to display these two buffers in the existing frame using two separate windows and we want that even if we told it elsewhere that we don't like tiny windows. Something like that. 😜

[gcv]

I can't reproduce the frame-opening behavior described in the Reddit thread in a sandbox environment (i.e., just Magit with its dependencies installed, no user customizations at all), no matter how small I make the frames. Tried Emacs 26.3 and a (slightly old) build of 27 on Mac OS. Some people on the thread use tiling WMs, could this somehow be a contributing factor? (It shouldn't but you never know.)

@acowley, since you have this problem reproduced, could you please try in a sandbox, or narrow down which customizations cause this? I'm happy to help in crafting a patch for this.

[acowley]

@tarsius I'm glad to hear it's considered a regression! I did not mean to assume anything, but evidently misunderstood, "I don't think I can work up the motivation to deal with that right now." Apologies for jumping in.