Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

options --exit-0 and --select-1 cause unexpected exits when filtering #369

Closed
aswild opened this issue Oct 27, 2020 · 2 comments
Closed
Labels

Comments

@aswild
Copy link

aswild commented Oct 27, 2020

My impression of the --exit-0 and --select-1 options are that they're most useful when piping some custom command into sk, to essentially bypass the TUI and immediately exit if there's 0 or 1 lines piped into sk, and it does this as expected.

What's unexpected is that these options also cause an immediate exit when interactively filtering in the UI as soon as a query is entered that matches 0 or 1 items. This behavior is different than fzf and seems unwanted.

For example, selecting md files in the skim source repo, and then typing c in the query:

$ fd -e md | fzf --height 5 --select-1


> CHANGELOG.md
  1/3
> c

Because there's more than 1 result from fd, the UI opens as expected and I can query to narrow things down. With fzf, querying c filters down to the only matching option, but waits for the user to accept it. With sk, the same setup exits the UI immediately upon pressing c (and prints CHANGELOG.md). --exit=0 behaves similarly.

Is this intentional behavior, or a bug of the implementation for these options?

@aswild
Copy link
Author

aswild commented Oct 27, 2020

Since I like hacking on my tools (my main interest in skim over fzf in the first place because I prefer Rust to Go), I whipped together this quick patch that seems to make --select-1 and --exit-0 behave more like fzf.

I'm not sure whether this is a terrible hack or a reasonable path, but the idea here is to keep track of the most matching options that have ever been seen, and trigger the exit on that value rather than the current number of matched options during every heartbeat.

diff --git a/src/model.rs b/src/model.rs
index cfdcc81..3e08842 100644
--- a/src/model.rs
+++ b/src/model.rs
@@ -50,6 +50,7 @@ pub struct Model {
     query: Query,
     selection: Selection,
     num_options: usize,
+    max_matched: usize,
     select1: bool,
     exit0: bool,
     sync: bool,
@@ -153,6 +154,7 @@ impl Model {
             query,
             selection,
             num_options: 0,
+            max_matched: 0,
             select1: false,
             exit0: false,
             sync: false,
@@ -355,6 +357,7 @@ impl Model {
         env.clear_selection = ClearStrategy::Clear;
         self.item_pool.reset();
         self.num_options = 0;
+        self.max_matched = 0;
         self.restart_matcher();
     }
 
@@ -370,10 +373,11 @@ impl Model {
         let processed = reader_stopped && items_consumed && matcher_stopped;
         let num_matched = self.selection.get_num_options();
         if processed {
-            if num_matched == 1 && self.select1 {
+            self.max_matched = max(self.max_matched, num_matched);
+            if self.max_matched == 1 && self.select1 {
                 debug!("select-1 triggered, accept");
                 let _ = self.tx.send((Key::Null, Event::EvActAccept(None)));
-            } else if num_matched == 0 && self.exit0 {
+            } else if self.max_matched == 0 && self.exit0 {
                 debug!("exit-0 triggered, accept");
                 let _ = self.tx.send((Key::Null, Event::EvActAbort));
             } else {

@lotabout lotabout added the bug label Oct 27, 2020
@aswild
Copy link
Author

aswild commented Nov 2, 2020

Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants