PatrickF1 / fzf.fish

🔍🐟 Fzf plugin for Fish
MIT License
1.96k stars 78 forks source link

[Search variables] Always wrap preview #239

Closed jaminthorns closed 2 years ago

PatrickF1 commented 2 years ago

Hi, can you explain why?

jaminthorns commented 2 years ago

My bad! I should've written up an explanation for this PR.

My FZF_DEFAULT_OPTS very purposefully doesn't include --preview-window='wrap', as I generally find wrapped lines confusing when viewing files and diffs. That means that for the functions that fzf.fish defines, the preview is not going to wrap unless it's set for that command (either explicitly in the function or through one of the _opts variables).

For searching history and processes, we use --preview-window="bottom:<LINES>:wrap" (3 lines for the command, 4 lines for the process info). This is a sensible default because the preview content is likely going to be a single line or few lines (commands and process info) that could be long (which is why we wrap). I figure variable values are a similar situation, and I actually ran into this while previewing a database URL variable that was cut off.

Now that I explain this, though, do you think the preview window for searching variables should be the same as for searching history (--preview-window="bottom:3:wrap")?

PatrickF1 commented 2 years ago

Hmm I see. Do you mind sharing the variables where you need the wrapping? TBH in my daily use I have never encountered variables that are too long width or height wise to need wrapping or to know whether the window should be on the right or the bottom

jaminthorns commented 2 years ago

Well, I can't exactly share the database URL, since those are sensitive credentials 😅 It's something like this, though:

postgresql://jamin:hbcvosgjmu34vjsym1o198n7n1i2cyhz@an-aws-cluster.cluster-tfzz82dqrmew.us-east-2.rds.amazonaws.com:5432/some_database?application_name=jamins_sql_client

Which would get cut off, even though there's plenty of room in the preview window for it to wrap. There's other things too, like default argument lists such as FZF_DEFAULT_OPTS:

--reverse --no-info --preview-window=border-sharp --prompt='▶ ' --pointer='▶' --marker='•' --color=bg+:#44475A,gutter:-1,hl:cyan,hl+:cyan,info:blue,marker:blue,pointer:blue,prompt:green,border:blue,header:bright-black

Or LESS:

--RAW-CONTROL-CHARS --use-color --ignore-case --chop-long-lines --clear-screen --tilde --shift=.1

Of course, now that I'm perusing all these variable, I can see that it's nice to have a lot of vertical space for a variable preview, since variables in fish are lists, and each item is displayed on a new line. So, let's forget what I said about making the preview window the same as searching history (on the bottom and only 3 lines tall). I still see no downside to always make the variable preview wrap, though (which is all that this PR is proposing).

Also, I know I can set it myself through fzf_shell_vars_opts, I just think it's a sensible default to wrap for this preview. I won't be offended at all if you close the PR without merging, though. 😁

PatrickF1 commented 2 years ago

Thanks for explaining and opening the PR! I'll merge this!