helm follow mode does not respect 'no-other-window

[Jefftree]

What happened?

helm follow mode should respect the 'no-other-window parameter if a window is marked and not use the corresponding window. (Note: This is only a problem for follow mode, regular invocations of helm respect the parameter)

How to reproduce?

Enable follow mode and have a split with two windows. Mark one of the window's 'no-other-window parameter to true and run a helm command that uses follow mode in the other window. The window with the 'no-other-window parameter set is used by follow mode.

Helm Version

Master branch

Emacs Version

Emacs-27.1

OS

GNU/Linux

Relevant backtrace (if possible)

No response

Minimal configuration

• I agree using a minimal configuration

[thierryvolpiatto]

Jeffrey Ying ***@***.***> writes:

What happened? helm follow mode should respect the 'no-other-window parameter if a window is marked and not use the corresponding window.

The no-other-window parameter should affect only other-window command (C-x o) and few other functions also obey this. Why would helm-persistent-action should obey this as long as it (may) visit a buffer handled by this window only temporarily during the helm session?

…

-- Thierry

[Jefftree]

Please correct me if I'm wrong but usually no-other-window is marked to prevent the window from being used as a window by other packages. Usually the window has an awkward size that isn't suitable for use by other packages. For instance treemacs creates a very long and narrow window to the side that is marked as no-other-window, and it's not very useful to have a helm-persistent-action use the particular window.

I'm obviously not familiar with the internals, but things like helm-find-files-other-window already respect this parameter which make sense.

[thierryvolpiatto]

Jeffrey Ying ***@***.***> writes:

Please correct me if I'm wrong but usually no-other-window is marked to prevent the window from being used as a window by other packages.

Originally it is here just to prevent other-window, but yes it can be used as a marker to say don't jump to this window (often coupled with dedication).

Usually the window has an awkward size that isn't suitable for use by other packages. For instance treemacs creates a very long and narrow window to the side that is marked as no-other-window, and it's not very useful to have a helm-persistent-action use the particular window.

So I guess it is the issue (treemacs), right? If so I will try to install treemacs, reproduce and see what I can do.

I'm obviously not familiar with the internals, but things like helm-find-files-other-window already respect this parameter which make sense.

IIRC, this was to fix same issue with treemacs.

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.*Message ID: ***@***.***>

-- Thierry

[Jefftree]

Ah yes, usually coupled with dedication. The issue isn't with treemacs (although can certainly be reproduced with it), any window being marked as no-other-window or dedicated would still be used by helm-persistent-action, and I think the correct behavior should be to not use those windows?

[thierryvolpiatto]

Hello @Jefftree ,
is this patch working for you?
It should work in most (reasonable cases).

commit 624da24796918b2a9340310d01cbc14082c77084
Author: Thierry Volpiatto <thievol@posteo.net>
Date:   Sat Dec 3 10:52:51 2022 +0100

    Try to fix some use cases where the initial window (#2575)
    
    is not suitable for PA.

diff --git a/helm-core.el b/helm-core.el
index 92c33b01..c49292f9 100644
--- a/helm-core.el
+++ b/helm-core.el
@@ -7077,15 +7077,21 @@ splitting inconditionally, it is unused actually."
              (attr-val (if (eq attr 'persistent-action-if)
                            (funcall (assoc-default attr source) selection)
                          (assoc-default attr source)))
-             ;; If attr value is a cons, use its car as persistent function
-             ;; and its car to decide if helm window should be splitted.
+             ;; If attr value is a cons, use its car as persistent function.
              (fn       (if (and (consp attr-val)
                                 ;; maybe a lambda.
                                 (not (functionp attr-val)))
                            (car attr-val) attr-val))
+             ;; And its cdr to decide if helm window should be splitted.
              (no-split (and (consp attr-val)
                             (not (functionp attr-val))
                             (cdr attr-val)))
+             ;; Is next-window (from helm-window) a suitable window for PA?
+             (no-suitable-win
+              (helm-aand (not helm--buffer-in-new-frame-p)
+                         (next-window (helm-window) 1 helm-initial-frame)
+                         (or (window-dedicated-p it)
+                             (window-parameter it 'no-other-window))))
              (cursor-in-echo-area t)
              mode-line-in-non-selected-windows)
         (progn
@@ -7100,7 +7106,7 @@ splitting inconditionally, it is unused actually."
                 (if no-split
                     (helm-select-persistent-action-window :split 'never)
                   (helm-select-persistent-action-window
-                   :split (or split helm-onewindow-p)))
+                   :split (or split helm-onewindow-p no-suitable-win)))
                 (helm-log "helm-execute-persistent-action"
                           "current-buffer = %S" (current-buffer))
                 (let ((helm-in-persistent-action t)
@@ -7138,8 +7144,14 @@ The symbol `never' is kept for backward compatibility."
                                    (get-buffer-window-list helm-buffer))))
                  helm-persistent-action-display-window)
                 ((and helm--buffer-in-new-frame-p helm-initial-frame)
-                 (with-selected-frame helm-initial-frame (selected-window)))
-                ((and split (not (eq split 'never))) (split-window))
+                 (with-selected-frame helm-initial-frame
+                   (let ((win (selected-window)))
+                     (if (or (window-dedicated-p win)
+                             (window-parameter win 'no-other-window))
+                         (next-window win 1)
+                       win))))
+                ((and split (not (eq split 'never)))
+                 (split-window))
                 ((get-buffer-window helm-current-buffer))
                 (t (previous-window (selected-window) 1))))))
 

[thierryvolpiatto]

Here starting with helm-mini from Treemacs window and hitting C-j.

Here with (setq helm-use-frame-when-no-suitable-window t):

And here with (setq helm-use-frame-when-no-suitable-window nil):

[thierryvolpiatto]

Also, I didn't remember about helm-prevent-escaping-from-minibuffer. When it is non nil (the default) Helm set 'no-other-window' on all the visible windows, so this test is not possible, a reasonable test is to test for side windows and dedicated windows which should fit most cases.

[Jefftree]

This works great, thank you so much!