Cleanup :rev filter code

Improve style and reduce duplication

Author: christian-schilling

josh-core/src/filter/mod.rs

@@ -221,8 +221,8 @@ fn lazy_refs2(op: &Op) -> Vec<String> {
         }
         Op::Rev(filters) => {
             let mut lr = lazy_refs2(&Op::Compose(filters.iter().map(|(_, _, f)| *f).collect()));
-            lr.extend(filters.iter().filter_map(|(_, x, _)| {
-                if let LazyRef::Lazy(s) = x {
+            lr.extend(filters.iter().filter_map(|(_, nested, _)| {
+                if let LazyRef::Lazy(s) = nested {
                     Some(s.to_owned())
                 } else {
                     None
@@ -234,8 +234,8 @@ fn lazy_refs2(op: &Op) -> Vec<String> {
         }
         Op::Squash(Some(revs)) => {
             let mut lr = vec![];
-            lr.extend(revs.keys().filter_map(|x| {
-                if let LazyRef::Lazy(s) = x {
+            lr.extend(revs.keys().filter_map(|nested| {
+                if let LazyRef::Lazy(s) = nested {
                     Some(s.to_owned())
                 } else {
                     None
@@ -466,55 +466,30 @@ fn get_rev_filter(
 
     // First match wins - iterate in order
     for (match_op, filter_tip_ref, startfilter) in filters.iter() {
+        let filter_tip = if let LazyRef::Resolved(filter_tip) = filter_tip_ref {
+            filter_tip
+        } else {
+            return Err(josh_error("unresolved lazy ref"));
+        };
+        if match_op != &RevMatch::Default && !transaction.repo().odb()?.exists(*filter_tip) {
+            return Err(josh_error(&format!(
+                "`:rev(...)` with nonexistent OID: {}",
+                filter_tip
+            )));
+        }
         let matches = match match_op {
             RevMatch::AncestorStrict => {
                 // `<` - matches if commit is ancestor of tip AND commit != tip (strict)
-                let filter_tip = if let LazyRef::Resolved(filter_tip) = filter_tip_ref {
-                    filter_tip
-                } else {
-                    return Err(josh_error("unresolved lazy ref"));
-                };
-
-                if !transaction.repo().odb()?.exists(*filter_tip) {
-                    return Err(josh_error(&format!(
-                        "`:rev(...)` with nonexistent OID: {}",
-                        filter_tip
-                    )));
-                }
 
                 is_ancestor_of(transaction, commit_id, *filter_tip)? && commit_id != *filter_tip
             }
             RevMatch::AncestorInclusive => {
                 // `<=` - matches if commit is ancestor of tip OR commit == tip (inclusive)
-                let filter_tip = if let LazyRef::Resolved(filter_tip) = filter_tip_ref {
-                    filter_tip
-                } else {
-                    return Err(josh_error("unresolved lazy ref"));
-                };
-
-                if !transaction.repo().odb()?.exists(*filter_tip) {
-                    return Err(josh_error(&format!(
-                        "`:rev(...)` with nonexistent OID: {}",
-                        filter_tip
-                    )));
-                }
 
                 is_ancestor_of(transaction, commit_id, *filter_tip)?
             }
             RevMatch::Equal => {
                 // `==` - matches if commit == tip
-                let filter_tip = if let LazyRef::Resolved(filter_tip) = filter_tip_ref {
-                    filter_tip
-                } else {
-                    return Err(josh_error("unresolved lazy ref"));
-                };
-
-                if !transaction.repo().odb()?.exists(*filter_tip) {
-                    return Err(josh_error(&format!(
-                        "`:rev(...)` with nonexistent OID: {}",
-                        filter_tip
-                    )));
-                }
 
                 commit_id == *filter_tip
             }

josh-filter/src/persist.rs

@@ -924,18 +924,18 @@ fn from_tree2(repo: &git2::Repository, tree_oid: git2::Oid) -> Result<Op, String
                 let (match_op, lazy_ref) = if key == "_" {
                     // Default filter - no SHA needed
                     (RevMatch::Default, LazyRef::Resolved(git2::Oid::zero()))
-                } else if key.starts_with("<=") {
-                    let ref_str = &key[2..];
+                } else if let Some(ref_str) = key.strip_prefix("<=") {
                     (
                         RevMatch::AncestorInclusive,
                         LazyRef::parse(ref_str).map_err(|e| e)?,
                     )
-                } else if key.starts_with('<') {
-                    let ref_str = &key[1..];
-                    (RevMatch::AncestorStrict, LazyRef::parse(ref_str)?)
-                } else if key.starts_with("==") {
-                    let ref_str = &key[2..];
-                    (RevMatch::Equal, LazyRef::parse(ref_str)?)
+                } else if let Some(ref_str) = key.strip_prefix('<') {
+                    (
+                        RevMatch::AncestorStrict,
+                        LazyRef::parse(ref_str).map_err(|e| e)?,
+                    )
+                } else if let Some(ref_str) = key.strip_prefix("==") {
+                    (RevMatch::Equal, LazyRef::parse(ref_str).map_err(|e| e)?)
                 } else {
                     return Err(format!(
                         "rev: invalid key format, must start with '<', '<=', '==', or be '_': {}",

tests/filter/rev.t

@@ -50,18 +50,11 @@
   |/  
   * 9f0db868b59a422c114df33bc6a8b2950f80490b:a087bfbdb1a5bad499b40ccd1363d30db1313f54
 
-  $ josh-filter -s ":rev(ffffffffffffffffffffffffffffffffffffffff:prefix=x/y)" --update refs/heads/filtered
-  ERROR: Invalid workspace:
-  ----
-   --> 1:6
-    |
-  1 | :rev(ffffffffffffffffffffffffffffffffffffffff:prefix=x/y)
-    |      ^---
-    |
-    = expected rev_entry
-  
-  :rev(ffffffffffffffffffffffffffffffffffffffff:prefix=x/y)
-  ----
+  $ josh-filter -s ":rev(==ffffffffffffffffffffffffffffffffffffffff:prefix=x/y)" --update refs/heads/filtered
+  [5] :prefix=x
+  [5] :prefix=y
+  [10] sequence_number
+  ERROR: `:rev(...)` with nonexistent OID: ffffffffffffffffffffffffffffffffffffffff
   [1]
 
   $ josh-filter -s ":rev(975d4c4975912729482cc864d321c5196a969271:prefix=x/y)" --update refs/heads/filtered