Necessity of overriding display-buffer-alist

[ktfleming]

Hi! Thanks for this great package that I use dozens of times per day -- it's become an invaluable part of my workflow.

I'm curious about the let-binding on display-buffer-alist here:

emacs-run-command/run-command-core.el

Lines 74 to 75 in 43afe90

	(let ((display-buffer-alist
	'((".*" display-buffer-use-least-recent-window))))

Looking at the history it was introduced in e9420f1, but I'm not sure how/if it's necessary to achieve the feature when command is already running, switch to it (from the commit message). Do you think this binding is indeed necessary?

I ask because I was trying to use display-buffer-alist to control how the buffers created by run-command appear, and found that it wasn't working, which led me to this. Removing it did get me the behavior that I wanted, and I haven't come across any odd side-effects yet, but I'm wondering if I might encounter some. Or if it's not actually necessary, maybe this binding could be removed in run-command.

[bard]

Hi! Thanks for this great package that I use dozens of times per day -- it's become an invaluable part of my workflow.

I'm really glad to hear!

I ask because I was trying to use display-buffer-alist to control how the buffers created by run-command appear, and found that it wasn't working, which led me to this. Removing it did get me the behavior that I wanted, and I haven't come across any odd side-effects yet, but I'm wondering if I might encounter some. Or if it's not actually necessary, maybe this binding could be removed in run-command.

I added it (begrudgingly) because I was hitting situations where the commands would show up in the current rather than the "other" window. I should at least have added a comment because I can't off the top of my head remember the exact sequence of steps. But yes, it's a heavy-handed solution. I'll try to reproduce the issue and post back here, if you're using display-buffer-alist routinely you might know of a better way.

[ktfleming]

I see -- in my case I ended up doing this:

(defun run-command-buffer-p (buffer arg)
  "Return non-nil if BUFFER is a buffer for a `run-command' invocation."
  (with-current-buffer buffer
    ;; TODO: is there a better way to determine this?
    (derived-mode-p 'term-mode)))

(setq display-buffer-alist
      '((run-command-buffer-p
         (display-buffer-reuse-mode-window display-buffer-pop-up-window))))

This seems to get me the behavior I was looking for, which is that command invocations will reuse any existing window that's already displaying a term-mode buffer, or creating a new window if one doesn't exist. But it only works for me because I only use term-mode for run-command commands -- if I used "regular" term-mode buffers, this would probably be too general.

I wonder if one solution would be to have a way of determining whether a given buffer is one created by run-command and not some other term/vterm/compilation buffer, i.e. filling in that TODO above; that way all users could customize their own display-buffer-alist to have the commands appear how they like (me using what I have above, perhaps you use display-buffer-use-least-recent-window, etc).

[bard]

You could use:

(defun run-command-buffer-p (buffer arg)
  (boundp 'run-command--command-spec))

run-command--command-spec is internal, so no guarantees of stability, but I think having a function to check whether a buffer is run-command output is a good idea, I'll probably add one like the above.

Is the second parameter (arg) necessary?

I wonder if one solution would be to have a way of determining whether a given buffer is one created by run-command and not some other term/vterm/compilation buffer, i.e. filling in that TODO above; that way all users could customize their own display-buffer-alist to have the commands appear how they like (me using what I have above, perhaps you use display-buffer-use-least-recent-window, etc).

Customizability is important but a reliable default behavior has higher priority — what to do will depend on those scenarios I mentioned above (haven't got around to trying to reproduce them yet).

[bard]

Sorry @ktfleming, the sequence contained a mistake, delete-other-window was meant to be delete-window. Could you have another try following the edit?

[ktfleming]

@bard Sorry for the delay, this slipped my mind. With the updated instructions and without the display-buffer-alist override I do see the behavior you describe, and agree that it's not ideal.

[bard]

No worries!

Can you check whether this works for you?

@@ -72,9 +72,17 @@ See `run-command-recipes' for the expected format of COMMAND-SPEC."
           (when (map-contains-key command-spec :hook)
             (funcall (map-elt command-spec :hook))))))
     (let ((display-buffer-alist
-           '((".*" display-buffer-use-least-recent-window))))
+           (append
+            display-buffer-alist
+            '((run-command-buuffer-p display-buffer-use-least-recent-window)))))
       (display-buffer buffer))))
 
+(defun run-command-buffer-p (buffer &optional _arg)
+  "Tell whether the given BUFFER was created by `run-command'.
+
+`ARG' is unused and only defined due to be required by `display-buffer-alist'."
+  (boundp 'run-command--command-spec))
+
 ;;;; Internals
 
 (defvar-local run-command--command-spec nil

The idea is to temporarily add the fallback rule at the end of the display-buffer-alist, so if you previously added your own rule, it will take precedence.

If it works, I'll make a release with it soon.

[ktfleming]

That's working for me, in doing some testing it seems my custom display-buffer-alist rule is getting prioritized over the fallback. 👍
(You might have noticed but there's a typo in the patch: run-command-buuffer-p. Just pointing it out to prevent the possibility of having to make a bugfix release 🙂 )

[bard]

(You might have noticed but there's a typo in the patch: run-command-buuffer-p. Just pointing it out to prevent the possibility of having to make a bugfix release slightly_smiling_face )

Great catch. +1

I just pushed this and the vterm fix, MELPA should pick those up shortly.

Thank you for reporting and for helping to test and debug this!

[vkazanov]

Hi @bard ,

Are you sure tweaking display-buffer-alist is the right option after all? I had to patch it out in the latest stable version using =el-patch= to fix the behaviour.

Quoting the Zen of buffer display:

display-buffer-alist and display-buffer-base-action are user options—Lisp programs must never set or rebind them.

Great package, btw, use it hourly!

Thanks