WIP: ✨ Add --preview-selection, alt approach #593

Implements feature without duplicate KEEP_RUNNING code from #496. Also adds docs, completions and Changelog entry.

Nice! I tried this with my setup from #496 and it looks good.

@dnkl Check out the latest commit to see if it's more like what you had mind.

@markstos yeah, something like that. Now all that remains is removing the callback mechanism, and calling preview() directly from wayland.c (this is necessary to keep supporting multi-seat).

fuzzel will produce an output each time selection changes

This is little misleading since it won't trigger when selection is changed by the query

Also, it doesn't trigger on initial selection. So to "preview" initial selection user would have to to up or down and back.

Ready for re-testing. Here's what to test:

echo -e 'first\nsecond' | ./pkg/fuzzel/usr/bin/fuzzel --preview-selection --minimal-lines --dmenu --width 10

• Test initial selection
• Test changing selection by filtering
• Test that the same selection is not repeated. Only output when selection when changes.
• Test selecting by arrow keys
• Test final selection is still emited.

This currently fails to build:

../../wayland.c: In function ‘wl_touch_up’:
../../wayland.c:1288:21: error: too few arguments to function ‘execute_selected’; expected 4, have 3
 1288 | execute_selected(seat, false, -1);
 | ^~~~~~~~~~~~~~~~
../../wayland.c:575:1: note: declared here
 575 | execute_selected(struct seat *seat, bool as_is, int custom_success_exit_code, bool preview)
 | ^~~~~~~~~~~~~~~~

(also seen in the CI)

Looks like a bad rebase/merge caused a build failure. Fixed now.

Converting this back to a draft because I'm not sure the UX here is the best for handling previews. There's an alternate way to think about it over this issue request:

#428 (comment)

Instead of emitting the selection to STDOUT, we could instead directly call a command with it. That eliminates the need to use some wrapper script to handle the selections, and keeps STDOUT used exclusively for the final output. Ex:

## If {selection} is not present, we pass it as arg to this command
--selection-cmd=imv
## Or if {selection} is present, we assume this is a command with arguments
--selection-cmd=/bin/imv {selection}

Waiting now to gather some feedback before considering a change of course.

For current approach:

• initial selection ✅
• changing selection by filtering ❌
• that the same selection is not repeated. Only output when selection when changes. ✅
• selecting by arrow keys ✅
• final selection is still emited. ✅

changing selection by filtering repro:

1. initial: "first" printed
2. query "s": "second" printed
3. query "" (empty): first is selected but NOT printed
4. submit: "first" printed

For "--selection-cmd" approach, this is how fzf does it (--bind='focus:execute-silent:COMMAND). I just started learning advanced uses of fzf like a week ago so can't say much about which approach would be better for fuzzel.

I like to align with other projects where it makes sense, so thanks for the fzf reference. fzf supports a very complex combination of many events triggers and actions. I'm not sure we'll want to implement all those, but it does look like a flexible and extensible system without adding an explosion of options. So maybe should adopt their --bind syntax, even if we only have a minimal number of triggers and actions now.

I think having a way to define a custom command to execute for previews is a much better approach than printing to stdout. It always bothered me that there were no obvious way to distinguish previews from the final, selected item, other that "it's the last printed item before fuzzel exited". But if you want to do something different for that item (and more importantly, not doing whatever you do to preview items), you couldn't really do that.

As for how to implement it, I'm open for suggestions. I'm not sure it makes sense to implement a complex bind syntax when we don't have any other actions to bind. Perhaps go with a simple preview-command for now? It can always be converted to something else in the future.

I think having a way to define a custom command to execute for previews is a much better approach than printing to stdout.

Having considered the the options, I agree.

I'm not sure it makes sense to implement a complex bind syntax when we don't have any other actions to bind.

I also agree. Although fzf does that, I just don't see Fuzzel adding so many "hooks" for external actions like that.

Perhaps go with a simple --preview-command for now? It can always be converted to something else in the future.

Yes, but what about adopting the naming convention of JavaScript events, where they name event handles like "onFocus", "onMouseOver" and so forth. On this case, perhaps:

--on-select-command

This both clarifies that it's an command triggered by an event also sets a precedent for other other event handler commands would be named if we were to add any more.

On this case, perhaps:

--on-select-command

Sounds good to me :)

The color picking tool maybe a related user, but needs more testing to understand how it works.

https://github.com/tom-gora/gmenu

Because we are going to change the implementation significantly and because this already conflicts so badly, I'm going to close this PR, and we'll try again later with --on-selection-cmd.