Add support for fzf as a frontend UI

[bnprks]

EDIT: I think now that a companion tool is a better approach for this, so I've closed the pull request and made mcfly-fzf

This pull request adds an option to use fzf as the front-end UI, addressing issue #158

If the environment variable MCFLY_FZF is defined at init time, then the fzf UI will be bound to Ctrl+R rather than the mcfly interface

This adds fzf support by:

1. Make a command mcfly fzf which just prints the command history as text to stdout
2. Modify the binding code in the shell init scripts so that if MCFLY_FZF is defined then Ctrl+R will instead run a copy of fzf reading its input from the mcfly fzf command with reasonable UI defaults

The only limitations of note are:

1. The MCFLY_FZF variable is only evaluated at init time. Due to the way that bash Ctrl+R is handled, it's not clear how to make the Ctrl+R binding change if the user sets MCFLY_FZF after init.
   • It looks doable to have hot-reload of MCFLY_FZF in zsh and fish, but this binding in mcfly.bash seems hard to make support hot-reloading: https://github.com/cantino/mcfly/blob/master/mcfly.bash#L70-L76
2. Deletion is not supported within the fzf interface, though running mcfly search manually should work fine

[bnprks]

One additional limitation I have since thought of:

3. The statistics for selected autocompletes will not be updated

I will try implementing this over the weekend, which will require making the functionality of the History record_selected_from_ui function callable from a command-line invocation.

In general, I'd be happy to take input on whether this change is desirable for mcfly, and if so how to make it minimally invasive on the rest of the code base.

[cantino]

Hey @bnprks, I'm trying to decide if this feels like a good direction for mcfly. It feels like it adds unneeded complexity.

Does mcfly fzf print the entire command history— e.g., it just dumps the sqlite db? Would a better plan be to have a fzf shell wrapper that talks to sqlite? Install mcfly for the command recording, but override it's ctrl-r binding to use your script and fzf?

[bnprks]

Yes, mcfly fzf prints the whole command history (up to MCFLY_HISTORY_LIMIT). I've been using this on 10k+ command histories without trouble. I believe contextual_commands temporary table creation also requires a scan through the same amount of history on each mcfly search invocation.

Of course an external command would be able to similarly read the mcfly database without modifying the core mcfly code. In this case, I think two aspects mean the wrapper code will have to contain a large fraction of mcfly logic internally:

1. Providing the option to sort by mcfly's neural network ranking requires the code to evaluate neural network rankings
2. Marking the selected commands also requires access to some other database insertion logic

It would probably be feasible to implement this using mcfly as a dependency in order to pull in this mcfly logic rather than being part of the core mcfly binary.

[cantino]

Okay, I think I understand better now. So you want to add a mcfly command that prints the sorted, prioritized DB view to STDOUT. That's reasonable. I think you could do that (perhaps call it mcfly search --dump or something) and even allow it to take a partial command to optionally filter by. You could do this without changing any of the shell integration scripts, which is the complexity I'd like to avoid. Then you can have a separate repo / gist that contains the integration instructions for fzf to use mcfly search --dump as it's backend. How does this sound? Am I understanding your goals right?

[bnprks]

Yeah, I think you're understanding the goals right. Thinking about it more, it might be possible to split entirely into a new repo. I'll test it out before completely abandoning this pull request, but I think since mcfly is on crates.io it might work to have a mcfly-fzf binary that implements the fzf commands, with mcfly as a dependency. Then that separate repo could include both the keybindings and the database functionality and mostly avoid duplicating mcfly code.

I think the main question there would be whether mcfly's public API exposes all the needed functionality, but I think it does

[cantino]

I think you might need to expand the exposed API or add mcfly search --dump for that to work. I'm supportive of adding --dump and --limit or something (as needed by #276). That seems fine, I'd just rather avoid integrating fzf into the already complex shell bindings.

[danihodovic]

@cantino is it currently possible to pipe results between mcfly and fzf? Use mcfly as the search "backend" while fzf handles the UI.

[cantino]

@danihodovic, It isn't possible today, but with the new options to search that we're discussing, you could have a script run mcfly search "foo bar" and then pipe that output into fzf.

[bnprks]

My first tests on separating this into a new binary seem promising -- basically all rust functions called in main.rs can be accessed publicly by a 3rd-party tool. The anticipated usage would be users run eval $(mcfly init bash), then run eval $(mcfly-fzf init bash), and there's a separate mcfly-fzf binary that contains the relevant functionality to read + dump the database.

The only change that would be needed in the main mcfly repo for this approach is updating the schema versioning logic to throw an error if it encounters a schema that's too new relative to the mcfly version. This would prevent conflicts in case a user's mcfly binary version diverged substantially from their mcfly-fzf version.

[cantino]

@bnprks Do you think it would be best to add the mcfly search command to dump text so that you're not depending on an internal API?

[lilianmoraru]

Please take https://github.com/lotabout/skim into consideration as well, so it's not completely tied into fzf. Maybe it helps you decide on a better method of wrapping these and other/future tools.

[cantino]

I assume skim would be able to handle dumping text from mcfly search foo --raw or similar as well? In which case, that supports my desire to do it that way.

[bnprks]

I've had success making a companion tool to mcfly here: https://github.com/bnprks/mcfly-fzf

I think compatibility with mcfly should be fairly straightforward to maintain even if the mcfly rust API changes. The only point of direct interaction between the mcfly binary and the mcfly-fzf binary is through the sqlite3 database. Given that the mcfly db schema has not changed in 4 years I think keeping mcfly-fzf in sync with future schema changes should not be too challenging.

I think it is most logical to have a companion tool specialized to providing the fzf interface. Being able to specialize the output and functionality to fzf's CLI options has been quite valuable and I'm not sure a generic text-dump option in mcfly would be able to provide as good a user experience without becoming tied to assumptions about how fzf works specifically.

Of course I'd be happy to help port some changes into the main mcfly binary if desired, but the companion-tool approach might scale better if people want to add support for other fuzzy search CLIs in the future.

[cantino]

Nice work @bnprks!