Remove $_fzf_search_vars_command

[kidonng]

This variable has been renamed in v7.0 which was released almost 3 months ago, people should have no dependence on it in their own config.

It is only used once in the user-facing code, i.e. fzf_configure_bindings. It can be setup for tests separately, no need to make it a global.

[PatrickF1]

There are two ideals in tension here:

1. make sure the tests make use of the actual code being run, rather than running a copy of it so we have lean on and trust tests to catch bugs and not have to worry about keeping both in sync
2. not leaking data into the user’s environment that they won’t use

As a maintainer who is short on time and updates this plugin rather infrequently so often lacking context, to me (1) is more important. (2) is important too of course, but it is mitigated by the fact that the name has an _ prepended to it, which universally means private. Also, fzf.fish is not the only plugin to leak this information. Looking at the output of set | rg \_, Fisher, Tide, and even fish itself all leak some global variables not meant to be used by users as well.

Sorry, but I’m going to have to say no to this idea.

[kidonng]

Also, fzf.fish is not the only plugin to leak this information. Looking at the output of set | rg \_, Fisher, Tide, and even fish itself all leak some global variables not meant to be used by users as well.

This is not the same. Those variables are global because they are shared by runtime code, but $_fzf_search_vars_command is only used once in the runtime code and made global solely for tests.

1. make sure the tests make use of the actual code being run

This is the only thing I'm concerned with, but unfortunately we can only choose one. What support me is that I have seen many projects running fine despite having tests with these "manually configured environments" instead of reusing the code.

[PatrickF1]

This is not the same. Those variables are global because they are shared by runtime code, but $_fzf_search_vars_command is only used once in the runtime code and made global solely for tests.

Touche, that's probably true. But it still does set the precedent of setting global variables users shouldn't interface with. Either way, I still prefer to just keep setting it as a global variable.