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

[Bug] alternate preview layout maintains separate hidden state #4100

Open
5 of 10 tasks
ibhagwan opened this issue Nov 18, 2024 · 2 comments
Open
5 of 10 tasks

[Bug] alternate preview layout maintains separate hidden state #4100

ibhagwan opened this issue Nov 18, 2024 · 2 comments

Comments

@ibhagwan
Copy link

Checklist

  • I have read through the manual page (man fzf)
  • I have searched through the existing issues
  • For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.56.2

OS

  • Linux
  • macOS
  • Windows
  • Etc.

Shell

  • bash
  • zsh
  • fish

Problem / Steps to reproduce

Consider the below command:

echo "line1\nline2" | fzf --preview 'echo $FZF_PREVIEW_COLUMNS' --preview-window 'right,60%,<80(down:45%)' --preview-window 'nohidden'

Run the command in any terminal width and use a keybind to hide the preview:

  • If the preview was down extend the terminal width until preview is shown on the right
  • If the preview was right decrease the terminal width until preview is shown at the bottom

It's very easy to do if you have a tmux split into two panes and then use <tmux-prefix>-z to "zoom" the pane which extends it to full terminal width.

It seems that the hidden state is saved per layout (horizontal/veritcal), when extending the terminal width the preview will lose its hidden state and will be un-hidden, when decreasing the terminal width again the preview will be hidden again.

ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Nov 18, 2024
Use alternative preview layout instead of using a "transform",
simpler, and we don't need to take into account the hidden state of
the preview as well as decrease the minimum fzf version for native
fzf flex layout to 0.31 (instead of 0.46).

Has known issue upstream where if the layout changes upon resize
the hidden state of the preview is lost:
junegunn/fzf#4100

Closes #1527
ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Nov 18, 2024
Use alternative preview layout instead of using a "transform",
simpler, and we don't need to take into account the hidden state of
the preview as well as decrease the minimum fzf version for native
fzf flex layout to 0.31 (instead of 0.46).

Has known issue upstream where if the layout changes upon resize
the hidden state of the preview is lost:
junegunn/fzf#4100

Closes #1527
ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Nov 18, 2024
Use alternative preview layout instead of using a "transform",
simpler, and we don't need to take into account the hidden state of
the preview as well as decrease the minimum fzf version for native
fzf flex layout to 0.31 (instead of 0.46).

Has known issue upstream where if the layout changes upon resize
the hidden state of the preview is lost:
junegunn/fzf#4100

Closes #1527
ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Nov 18, 2024
Use alternative preview layout instead of using a "transform",
simpler, and we don't need to take into account the hidden state of
the preview as well as decrease the minimum fzf version for native
fzf flex layout to 0.31 (instead of 0.46).

Has known issue upstream where if the layout changes upon resize
the hidden state of the preview is lost:
junegunn/fzf#4100

Closes #1527
ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Nov 18, 2024
Use alternative preview layout instead of using a "transform",
simpler, and we don't need to take into account the hidden state of
the preview as well as decrease the minimum fzf version for native
fzf flex layout to 0.31 (instead of 0.46).

Has known issue upstream where if the layout changes upon resize
the hidden state of the preview is lost:
junegunn/fzf#4100

Closes #1527
ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Nov 19, 2024
Use alternative preview layout instead of using a "transform",
simpler, and we don't need to take into account the hidden state of
the preview as well as decrease the minimum fzf version for native
fzf flex layout to 0.31 (instead of 0.46).

Has known issue upstream where if the layout changes upon resize
the hidden state of the preview is lost:
junegunn/fzf#4100

Closes #1527
ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Nov 19, 2024
Use alternative preview layout instead of using a "transform",
simpler, and we don't need to take into account the hidden state of
the preview as well as decrease the minimum fzf version for native
fzf flex layout to 0.31 (instead of 0.46).

Has known issue upstream where if the layout changes upon resize
the hidden state of the preview is lost:
junegunn/fzf#4100

Closes #1527
ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Nov 19, 2024
Use alternative preview layout instead of using a "transform",
simpler, and we don't need to take into account the hidden state of
the preview as well as decrease the minimum fzf version for native
fzf flex layout to 0.31 (instead of 0.46).

Has known issue upstream where if the layout changes upon resize
the hidden state of the preview is lost:
junegunn/fzf#4100

Closes #1527
@junegunn
Copy link
Owner

junegunn commented Nov 23, 2024

I know it's not ideal but it's the intended behavior. You can read more about it in #3280 (comment).

The behavior was introduced when I tried to fix #3113. While working on a fix, I had a hard time making sense of the toggling behavior w.r.t. alternative layout whose hidden property is not equal to that of the default layout. So I decided that it's easier to understand the state transition if the hidden property is managed separately by each layout.

Here's one example:

fzf --preview-window '<50(hidden)' --bind space:toggle-preview --preview 'cat {}'
  • The intention seems pretty clear. You don't want to see the preview window on a narrow screen.
  • If you shrink the screen to below 50 columns, the preview window gets hidden.
  • If you enlarge it again, the preview window reappears.
  • Now, let's say there's one global "visibility" state, and you hit space twice to hide and re-show the preview window.
  • What should happen when you shrink the window again? Should it be hidden or not? How do we interpret the intention of the user?

There were some other different scenarios where I couldn't really answer the question with confidence. So let's see it this way, we support two distinct sets of options for different screen sizes and they don't affect each other. When the screen resizes, fzf sees that the situation has changed and the previous states are ignored in this new situation. This isn't ideal, of course, and it may run counter to user expectations, but I felt it's easier to reason about.

We could try to find a better heuristic to reconcile the global "visibility" state with the "hidden" property of each layout, but it's quite tricky, especially when considering change-preview-window that can change both layouts dynamically.

So I'd say this is not a bug, but we have room for improvement.

Related: #3280

@ibhagwan
Copy link
Author

Ty for the detailed explanation @junegunn.

The only issue I have this with is that if I try to use change-preview-window with a transform I cannot solve this due to not having access to the current hidden state as explained in #4098.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants