Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent behavior of splitting window on TAB (select actions) #2635

Closed
1 task done
rdiaz02 opened this issue Jan 17, 2024 · 15 comments
Closed
1 task done

Inconsistent behavior of splitting window on TAB (select actions) #2635

rdiaz02 opened this issue Jan 17, 2024 · 15 comments
Labels

Comments

@rdiaz02
Copy link

rdiaz02 commented Jan 17, 2024

What happened?

On wide frames, when we TAB, the actions are shown in a window that hides the previous helm window.
h-w

On narrower frames, when we TAB there is a split and both windows are shown.
h-narrow

This is also the behavior even in very narrow frames.

h-v-arrow

I would have expected exactly the opposite (split, except if too narrow). Is this the intended behavior?

I can get splitting on wide frames by using very large values of split-width-threshold (not of helm-split-width-threshold) ; but on narrow frames, we still get the split (where, I think, there should be no split).

Interestingly, if I use

(setq helm-always-two-windows nil)
(setq helm-default-display-buffer-functions
      '(display-buffer-in-side-window))

regardless of the width of the frame, the actions window always hides the previous helm window.

How to reproduce?

Start emacs -Q and then

(package-initialize)
(helm-mode)

Next, M-x helm-find-files and then TAB)

Helm Version

Melpa or NonGnuElpa

Emacs Version

Emacs-29.1

OS

GNU/Linux

Relevant backtrace (if possible)

No response

Minimal configuration

  • I agree using a minimal configuration
@rdiaz02 rdiaz02 added the bug label Jan 17, 2024
@rdiaz02 rdiaz02 changed the title Inconsistent behavior of splitting window on TAB (list-actions Inconsistent behavior of splitting window on TAB (list-actions) Jan 17, 2024
@rdiaz02 rdiaz02 changed the title Inconsistent behavior of splitting window on TAB (list-actions) Inconsistent behavior of splitting window on TAB (select actions) Jan 17, 2024
@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Jan 17, 2024 via email

thierryvolpiatto added a commit that referenced this issue Jan 17, 2024
We were previously checking split-witdth-threshold which is now let-bounded to
helm-split-width-threshold (was not existing at the time), check
instead helm-split-width-threshold.
In addition a check of helm-split-window-default-side is done which
IMO make sense as well.
thierryvolpiatto added a commit that referenced this issue Jan 17, 2024
when helm-show-action-window-other-window is left or right and window
is smaller that split-width-threshold (use 160, its default value when
set to nil).
@rdiaz02
Copy link
Author

rdiaz02 commented Jan 17, 2024

Thanks a lot. That works perfectly. And I see I can control the behavior using helm-split-width-threshold.
However, if I have

(setq helm-always-two-windows nil)
(setq helm-default-display-buffer-functions
      '(display-buffer-in-side-window))

then, regardless of the width of the frame, the action list window always hides the previous helm window.

(Why am I using the above settings? To have helm always be displayed on the bottom side window).

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Jan 17, 2024 via email

@rdiaz02
Copy link
Author

rdiaz02 commented Jan 17, 2024

Oh, sorry, I had misunderstood. Now I have

(setq helm-always-two-windows t)
(setq helm-default-display-buffer-functions
      '(display-buffer-in-side-window))

But now I get the error split-window: Cannot split side window or parent of side window when I press TAB. In fact, it is the same error I remember getting when I tried adding helm windows in display-buffer-alist with (display-buffer-in-side-window).

Isn't setting helm-split-window-default-side enough?

I want to have the helm window to always appear at the bottom of the frame, not below each window (because I find it is easier for me to know where to look for it). This issue was discussed in #2039 a while back.

But maybe this is a completely minor issue that doesn't affect anyone else, and it is not worth your time.

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Jan 17, 2024 via email

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Jan 18, 2024

I have now made modifications so that action menu is shown even in vertical split (2dc2efc), e.g. After splitting vertically with C-t and hitting TAB the action menu is shown below. So I wonder if the helm-always-two-windows limitation is still necessary, perhaps i will remove it soon.

thierryvolpiatto added a commit that referenced this issue Jan 18, 2024
with helm-show-action-window-other-window.
Same with helm--buffer-in-new-frame-p test which is not needed now we
check the size of window.
@rdiaz02
Copy link
Author

rdiaz02 commented Jan 18, 2024

Thanks!

Where at the bottom of the frame? helm-always-two-windows always
display the helm-buffer below all other windows but horizontally.

At the bottom of the complete frame, below all of the windows, so that the splits are not changed, except for the new bottom window.

For example, this is one initial window configuration

h01

If I do helm-find-files in the bottom right window, the window layout gets completely changed:

h02

But I'd rather have something like this, where the new helm windows always appear at the bottom of the frame, so what happens is that the rest of the windows are compressed vertically, but the rest of the splits are not modified, and all the windows that were originally visible are still visible (sorry, I seem unable to reproduce what I would like with emacs -Q and the currently available options):

h03

Right now, everything seems to do exactly what I want (the last screen capture). But I am not sure why/how: I've cheated there and used shackle (https://depp.brause.cc/shackle/). But the author of shackle seems to suggest, in https://emacsninja.com/posts/design-is-hard.html, that shackle is a wrapper around display-buffer-alist , but less powerful and harder to debug. Thus, I think I should be able to achieve what I want directly modifying display-buffer-alist , but the fact that I can't tells me that there is something I am not understanding about display-buffer-alist. But this is now about a different issue.

As I said, I really appreciate your help, but this is is probably taking a lot of your time for something that might be relevant to just one user. And the minor bug of the original issue has been fixed.

@thierryvolpiatto
Copy link
Member

Is it what you want? See the config in scratch buffer.
Capture d’écran_2024-01-18_13-28-55

@thierryvolpiatto
Copy link
Member

Here is how display-buffer is called in Helm:

(display-buffer
       buffer `(,helm-default-display-buffer-functions
                . ,(append helm-default-display-buffer-alist
                           `((window-height . ,helm-display-buffer-default-height)
                             (window-width  . ,helm-display-buffer-default-width)))))

@thierryvolpiatto
Copy link
Member

I made a mistake with helm-display-buffer-height, here the right setting:

(setq helm-display-buffer-height 'fit-window-to-buffer)

@thierryvolpiatto
Copy link
Member

And here a screenshot using default settings with helm-use-frame-when-more-than-two-windows (helm is displayed in a frame):

Capture d’écran_2024-01-18_13-48-05

@thierryvolpiatto
Copy link
Member

Argh! here finally the right setting for what you asked:

(setq helm-always-two-windows nil)

(setq helm-default-display-buffer-functions
      '(display-buffer-at-bottom))

(setq helm-use-frame-when-more-than-two-windows nil)

(setq helm-display-buffer-default-height 20)

(setq helm-default-display-buffer-alist nil)

Capture d’écran_2024-01-18_13-59-45

@rdiaz02
Copy link
Author

rdiaz02 commented Jan 19, 2024

Thanks a lot!!!!! That works great!!!!!

@rdiaz02
Copy link
Author

rdiaz02 commented Jan 19, 2024

I guess we can close this issue?

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Jan 19, 2024 via email

@rdiaz02 rdiaz02 closed this as completed Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants