junegunn / fzf

:cherry_blossom: A command-line fuzzy finder
https://junegunn.github.io/fzf/
MIT License
65.84k stars 2.41k forks source link

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

Open ibhagwan opened 1 week ago

ibhagwan commented 1 week ago

Checklist

Output of fzf --version

0.56.2

OS

Shell

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:

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.

junegunn commented 4 days ago

I know it's not ideal but it's the intended behavior. You can read more about it in https://github.com/junegunn/fzf/issues/3280#issuecomment-1535720030.

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 {}'

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 commented 3 days ago

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.