fix(matcher): an inverse query only checks the first --nth field (#1159)

Co-authored-by: VXNCXNX <vxncxnx@users.noreply.github.com>

src/engine/exact.rs

@@ -79,24 +79,33 @@ impl MatchEngine for ExactEngine {
         let mut matched_result = None;
         let item_text = item.text();
         let default_range = [(0, item_text.len())];
-        for &(start, end) in item.get_matching_ranges().unwrap_or(&default_range) {
-            let start = min(start, item_text.len());
-            let end = min(end, item_text.len());
-            if self.query_regex.is_none() {
-                matched_result = Some((0, 0));
-                break;
+        let ranges = item.get_matching_ranges().unwrap_or(&default_range);
+
+        if ranges.is_empty() {
+            // Nothing to match against (e.g. every `--nth` index is out of range): the item
+            // stays unmatched, inverse or not.
+        } else if self.query_regex.is_none() {
+            matched_result = Some((0, 0));
+        } else {
+            for &(start, end) in ranges {
+                let start = min(start, item_text.len());
+                let end = min(end, item_text.len());
+
+                matched_result =
+                    regex_match(&item_text[start..end], self.query_regex.as_ref()).map(|(s, e)| (s + start, e + start));
+
+                if matched_result.is_some() {
+                    break;
+                }
             }
 
-            matched_result =
-                regex_match(&item_text[start..end], self.query_regex.as_ref()).map(|(s, e)| (s + start, e + start));
-
+            // An inverse query has to be evaluated over *all* the matching ranges: the item
+            // only matches when none of them contains the query.  Inverting inside the loop
+            // would let the first non-matching field short-circuit the scan and wrongly
+            // accept an item whose later fields do contain the query.
             if self.inverse {
                 matched_result = matched_result.xor(Some((0, 0)));
             }
-
-            if matched_result.is_some() {
-                break;
-            }
         }
 
         let (begin, end) = matched_result?;

src/engine/exact_tests.rs

@@ -83,6 +83,83 @@ fn inverse_match_excludes_query() {
     assert!(e.match_item(&"foo".to_string()).is_none());
 }
 
+/// An item exposing explicit matching ranges, as `--nth` produces.
+struct RangedItem {
+    text: String,
+    ranges: Vec<(usize, usize)>,
+}
+
+impl SkimItem for RangedItem {
+    fn text(&self) -> std::borrow::Cow<'_, str> {
+        std::borrow::Cow::Borrowed(&self.text)
+    }
+
+    fn get_matching_ranges(&self) -> Option<&[(usize, usize)]> {
+        Some(&self.ranges)
+    }
+}
+
+#[test]
+fn inverse_match_checks_every_matching_range() {
+    // `--nth 1,2` over "foo bar" yields two ranges: "foo" and "bar".  An inverse
+    // query `!foo` must reject the item because one of the ranges contains "foo",
+    // even though the *first* range scanned may not.
+    let e = engine(
+        "foo",
+        ExactMatchingParam {
+            inverse: true,
+            case: CaseMatching::Ignore,
+            ..Default::default()
+        },
+    );
+
+    let foo_in_first_range = RangedItem {
+        text: "foo bar".to_string(),
+        ranges: vec![(0, 3), (4, 7)],
+    };
+    assert!(
+        e.match_item(&foo_in_first_range).is_none(),
+        "item whose first field contains the query must not match an inverse query"
+    );
+
+    let foo_in_second_range = RangedItem {
+        text: "bar foo".to_string(),
+        ranges: vec![(0, 3), (4, 7)],
+    };
+    assert!(
+        e.match_item(&foo_in_second_range).is_none(),
+        "item whose second field contains the query must not match an inverse query"
+    );
+
+    let no_foo = RangedItem {
+        text: "bar baz".to_string(),
+        ranges: vec![(0, 3), (4, 7)],
+    };
+    assert!(
+        e.match_item(&no_foo).is_some(),
+        "item where no field contains the query must match an inverse query"
+    );
+}
+
+#[test]
+fn inverse_match_with_no_matching_range_does_not_match() {
+    // Every `--nth` index out of range leaves the item with no range at all;
+    // there is nothing to match against, so the item stays unmatched.
+    let e = engine(
+        "foo",
+        ExactMatchingParam {
+            inverse: true,
+            case: CaseMatching::Ignore,
+            ..Default::default()
+        },
+    );
+    let item = RangedItem {
+        text: "bar baz".to_string(),
+        ranges: vec![],
+    };
+    assert!(e.match_item(&item).is_none());
+}
+
 #[test]
 fn empty_query_matches_everything() {
     let e = engine("", ExactMatchingParam::default());

tests/cli.rs

@@ -110,6 +110,28 @@ fn filter_mode_with_print0() {
     assert!(stdout.contains('\0'));
 }
 
+#[test]
+fn filter_mode_inverse_query_checks_every_nth_field() {
+    // `--nth 1,2` gives each item two matching ranges. An inverse query must
+    // reject an item when ANY of them contains the term, not just the first one.
+    let (code, stdout, _) = run_sk_argv("foo bar\nqux bar\n", &["-f", "!foo", "--nth", "1,2"], &[]);
+    assert_eq!(code, Some(0));
+    assert!(
+        !stdout.contains("foo bar"),
+        "!foo must exclude 'foo bar' (got {stdout:?})"
+    );
+    assert!(stdout.contains("qux bar"), "!foo must keep 'qux bar' (got {stdout:?})");
+
+    // The term sitting in the second field must be caught too.
+    let (code, stdout, _) = run_sk_argv("bar foo\nbar qux\n", &["-f", "!foo", "--nth", "1,2"], &[]);
+    assert_eq!(code, Some(0));
+    assert!(
+        !stdout.contains("bar foo"),
+        "!foo must exclude 'bar foo' (got {stdout:?})"
+    );
+    assert!(stdout.contains("bar qux"), "!foo must keep 'bar qux' (got {stdout:?})");
+}
+
 #[test]
 fn filter_mode_no_sort_preserves_input_order() {
     // Workers grab 4096-item chunks from a shared queue, so with enough items