feat: allow configuring fzf options

[CBeerta]

Which issue does this PR close?

Closes #2715

Rationale of this PR

[sxyazi]

fzf has about 100 options, and we can't possibly support each one individually.

So please consider a more general solution by forwarding any parameters provided by the user to fzf, not just --walker here.

[CBeerta]

Yeah, ofc. Is this better? I haven't actually ever really written any lua.

Or would you rather use a --args="--walker=hidden" style option?

[Shallow-Seek]

I am writing a plugin that allows you to configure the search command fd, rg, etc., and all fzf options in one config file beside the main.lua so you can populate different fzf utilities in yazi. I will post it in a few days.

[XYenon]

fzf has about 100 options, and we can't possibly support each one individually.

So please consider a more general solution by forwarding any parameters provided by the user to fzf, not just --walker here.

Do you think this is acceptable? XYenon@1fd7338

[sxyazi]

The issue is trickier than expected because fzf requires its arguments to be passed as an ordered list, while Yazi plugin arguments are an unordered dict.

This means that something like fzf --multi --no-multi --multi can't be represented, since the order of multi and no-multi in a dictionary is not guaranteed.

Also, fzf supports argument shorthands, such as fzf --multi --no-multi -m +m, which also can't be represented - where -m is equivalent to --multi, and +m is equivalent to --no-multi.

Furthermore, if plugin arguments are treated as fzf arguments, it's unclear how to properly pass arguments of the plugin itself.

[shimeoki]

The issue is trickier than expected because fzf requires its arguments to be passed as an ordered list, while Yazi plugin arguments are an unordered dict.

then can't we just do the same as in the zoxide plugin - pass the arguments to fzf via an environment variable?

yazi/yazi-plugin/preset/plugins/zoxide.lua

Lines 44 to 48 in a3e9ea0

	return (os.getenv("FZF_DEFAULT_OPTS") or "")
	.. " "
	.. table.concat(default, " ")
	.. " "
	.. (os.getenv("YAZI_ZOXIDE_OPTS") or "")

but use something like YAZI_FZF_OPTS.

it's less flexible than the programmable way with the lua options, but at least still gives the freedom to the user to modify the arguments. and still probably covers 90% of use cases.

edit:

only after some thought i realized that's not easy as well, because in case of zoxide the environment variable is just assigned to pass the arguments, but in case of fzf these arguments need to be converted to the format that is accepted in :arg or :args.

still, i think it's possible to just get the environment variable, parse it to lua list and then pass it to the :args.

also, why this PR modifies the magick plugin?

---

also, it's not specific to the current pull request, because it's related to the zoxide plugin, but a code block from this plugin was already mentioned above, so i want to write about it here (if it's approved, i can make a PR):

i believe the usage of FZF_DEFAULT_OPTS in the zoxide plugin is meaningless, because fzf uses the default options unconditionally if one of the environment variables is set (FZF_DEFAULT_OPTS or FZF_DEFAULT_OPTS_FILE), so zoxide should already use fzf with the default opts in any case.

and i think the zoxide plugin should allow disabling the default options and/or allow disabling reassign of the _ZO_FZF_OPTS. for example, i defined my own _ZO_FZF_OPTS for the interactive zoxide in the cli, and expected this behavior in yazi as well, but default options override this environment variable.

yazi/yazi-plugin/preset/plugins/zoxide.lua

Line 96 in a3e9ea0

:env("_ZO_FZF_OPTS", options())

and even if i define my _ZO_FZF_OPTS as YAZI_ZOXIDE_OPTS, then my preview window layout is different, because the "default" options that are generated by yazi and passed to the fzf cli still override my default opts. i think one of the solutions would be to read the options from the FZF_DEFAULT_OPTS_FILE to the YAZI_ZOXIDE_OPTS, but it seems unnecessary complex.

[sxyazi]

then can't we just do the same as in the zoxide plugin - pass the arguments to fzf via an environment variable?

only after some thought i realized that's not easy as well

Yeah we can't easily do so, because that would be a global config. This PR is intended to solve #2715, which is to allow passing different fzf parameters for each key binding. The current problem is that the plugin API can only get key parameters as an unordered dictionary, not an ordered list, whereas fzf requires an ordered list.

i believe the usage of FZF_DEFAULT_OPTS in the zoxide plugin is meaningless, because fzf uses the default options unconditionally if one of the environment variables is set (FZF_DEFAULT_OPTS or FZF_DEFAULT_OPTS_FILE), so zoxide should already use fzf with the default opts in any case.

Nope, once you pass _ZO_FZF_OPTS, zoxide will no longer respect the default config of fzf, and the necessary parameters that zoxide itself sets for fzf will also be lost, because zoxide will completely override FZF_DEFAULT_OPTS and not pass along the parameters required by zoxide itself - this is an unresolved issue ajeetdsouza/zoxide#618

Hence, Yazi must merge FZF_DEFAULT_OPTS (line 44) with zoxide's default parameters (line 46), and keep a YAZI_ZOXIDE_OPTS variable to allow users to override all previous parameters (line 48). What this code basically does is to reproduce ajeetdsouza/zoxide#618 (comment), that is, a "_ZO_FZF_OPTS" option that respects the user's fzf config and zoxide's default config (with some tweaks to make it consistent with Yazi's UI, e.g. height 100%, rounded edges).

I might add some comments in the code later to clarify this behavior and the reasons for doing so

[shimeoki]

Nope, once you pass _ZO_FZF_OPTS, zoxide will no longer respect the default config of fzf, and the necessary parameters that zoxide itself sets for fzf will also be lost, because zoxide will completely override FZF_DEFAULT_OPTS and not pass along the parameters required by zoxide itself - this is an unresolved issue ajeetdsouza/zoxide#618

oh. i see. i avoided this issue unintentionally by using FZF_DEFAULT_OPTS_FILE.

Hence, Yazi must merge FZF_DEFAULT_OPTS (line 44) with zoxide's default parameters (line 46), and keep a YAZI_ZOXIDE_OPTS variable to allow users to override all previous parameters (line 48). What this code basically does is to reproduce ajeetdsouza/zoxide#618 (comment), that is, a "_ZO_FZF_OPTS" option that respects the user's fzf config and zoxide's default config (with some tweaks to make it consistent with Yazi's UI, e.g. height 100%, rounded edges).

i forgot that if the code was written, there is always a reason for that. sorry, should have done my research.

Yeah we can't easily do so, because that would be a global config. This PR is intended to solve #2715, which is to allow passing different fzf parameters for each key binding. The current problem is that the plugin API can only get key parameters as an unordered dictionary, not an ordered list, whereas fzf requires an ordered list.

then, does the yazi plugin api support passing quoted arguments? if so, then we can do a hack to pass the arguments to fzf as a single argument to the plugin. either positional or use a specific name (args, pass, etc.):

plugin fzf -- "--multi --height=100%"
plugin fzf -- --args='--ignore-case --no-sort'

though it's a hack, i think it's fine. support from the api level to get the arguments in order would be great, but as you mentioned, there should probably be a separation between fzf arguments and fzf plugin arguments, so i'm against "mapping" them directly.

and then this string is passed as FZF_DEFAULT_OPTS environment variable to the executed command, where FZF_DEFAULT_OPTS is prepended to the resulting env. again, something like zoxide plugin does.

[shimeoki]

i played around a bit.

yazi indeed supports quoted arguments. my implementation looks like this:

--- a/yazi-plugin/preset/plugins/fzf.lua
+++ b/yazi-plugin/preset/plugins/fzf.lua
@@ -8,13 +8,13 @@ local state = ya.sync(function()
 	return cx.active.current.cwd, selected
 end)
 
-function M:entry()
+function M:entry(job)
 	ya.emit("escape", { visual = true })
 
 	local _permit = ui.hide()
 	local cwd, selected = state()
 
-	local output, err = M.run_with(cwd, selected)
+	local output, err = M.run_with(cwd, selected, job.args)
 	if not output then
 		return ya.notify { title = "Fzf", content = tostring(err), timeout = 5, level = "error" }
 	end
@@ -29,13 +29,26 @@ function M:entry()
 	end
 end
 
-function M.run_with(cwd, selected)
-	local child, err = Command("fzf")
-		:arg("-m")
+function M.run_with(cwd, selected, args)
+	local fzf_args = (os.getenv("FZF_DEFAULT_OPTS") or "")
+		.. " "
+		.. table.concat({ "--multi" }, " ")
+		.. " "
+		.. (os.getenv("YAZI_FZF_OPTS") or "")
+		.. " "
+		.. (args.pass or "")
+
+	local cmd = Command("fzf")
+		:env("FZF_DEFAULT_OPTS", fzf_args)
 		:cwd(tostring(cwd))
 		:stdin(#selected > 0 and Command.PIPED or Command.INHERIT)
 		:stdout(Command.PIPED)
-		:spawn()
+
+	if args.command then
+		cmd:env("FZF_DEFAULT_COMMAND", args.command)
+	end
+
+	local child, err = cmd:spawn()
 
 	if not child then
 		return nil, Err("Failed to start `fzf`, error: %s", err)

and then it's used like this:

run = '''
    plugin fzf -- --pass='--black --preview="fzf-preview.bash {}" --no-multi'
'''

i added the additional --command plugin argument to support what was in the original issue.

the author wanted to use --walker option via a keybinding, but as said in fzf manual page, it Determines the behavior of the built-in directory walker that is used when $FZF_DEFAULT_COMMAND is not set..

so just using --walker=dir,follow does nothing, because in my case i have the FZF_DEFAULT_COMMAND set. and i think that's the case for most of the users.

with the approach above, the wanted result can be achieved like this:

run = "plugin fzf -- --pass='--walker=dir,follow' --command=''"

blank --command effectively unsets the environment variable in that particular case.

aside from that, i still added support for configuring the plugin via environment variable YAZI_FZF_OPTS (honestly, i just needed it, and it's the reason how i found this PR in the first place).

edit:

obviously, naming can be changed. maybe i like --opts more instead of --pass, because it aligns better with FZF_DEFAULT_OPTS.

and an environment variable to set FZF_DEFAULT_COMMAND also can be added, e.g. YAZI_FZF_COMMAND. can be helpful in situations if you only want to use default walker in yazi for some reason, idk.