Limit Filter "universes" to invocation::filter

Universes are really an optimization of filtering tasks, so let's define them there, and derive them from the set of conditions. This means that complex filters might get missed and end up doing a full task scan, but that's probably OK. Note that this does not fix the working-set API issues (#108 and #123).
2020-12-30 00:23:18 +00:00
parent 0a458b5f5b
commit fc977a0fe6
5 changed files with 236 additions and 147 deletions
--- a/cli/src/argparse/filter.rs
+++ b/cli/src/argparse/filter.rs
@@ -2,6 +2,7 @@ use super::args::{arg_matching, id_list, minus_tag, plus_tag, TaskId};
 use super::ArgList;
 use crate::usage;
 use nom::{branch::alt, combinator::*, multi::fold_many0, IResult};
+use taskchampion::Status;

 /// A filter represents a selection of a particular set of tasks.
 ///
@@ -10,40 +11,11 @@ use nom::{branch::alt, combinator::*, multi::fold_many0, IResult};
 /// pending tasks, or all tasks.
 #[derive(Debug, PartialEq, Default, Clone)]
 pub(crate) struct Filter {
-    /// The universe of tasks from which this filter can select
-    pub(crate) universe: Universe,
-
    /// A set of filter conditions, all of which must match a task in order for that task to be
    /// selected.
    pub(crate) conditions: Vec<Condition>,
 }

-/// The universe of tasks over which a filter should be applied.
-#[derive(Debug, PartialEq, Clone)]
-pub(crate) enum Universe {
-    /// Only the identified tasks.  Note that this may contain duplicates.
-    IdList(Vec<TaskId>),
-    /// All tasks in the task database
-    AllTasks,
-    /// Only pending tasks (or as an approximation, the working set)
-    #[allow(dead_code)] // currently only used in tests
-    PendingTasks,
-}
-
-impl Universe {
-    /// Testing shorthand to construct a simple universe
-    #[cfg(test)]
-    pub(super) fn for_ids(mut ids: Vec<usize>) -> Self {
-        Universe::IdList(ids.drain(..).map(|id| TaskId::WorkingSetId(id)).collect())
-    }
-}
-
-impl Default for Universe {
-    fn default() -> Self {
-        Self::AllTasks
-    }
-}
-
 /// A condition which tasks must match to be accepted by the filter.
 #[derive(Debug, PartialEq, Clone)]
 pub(crate) enum Condition {
@@ -52,10 +24,18 @@ pub(crate) enum Condition {

    /// Task does not have the given tag
    NoTag(String),
+
+    // TODO: add command-line syntax for this
+    /// Task has the given status
+    Status(Status),
+
+    /// Task has one of the given IDs
+    IdList(Vec<TaskId>),
 }

 /// Internal struct representing a parsed filter argument
 enum FilterArg {
+    // TODO: get rid of this whole enum
    IdList(Vec<TaskId>),
    Condition(Condition),
 }
@@ -67,30 +47,46 @@ impl Filter {
            Filter {
                ..Default::default()
            },
-            Self::fold_args,
+            |acc, arg| acc.with_arg(arg),
        )(input)
    }

    /// fold multiple filter args into a single Filter instance
-    fn fold_args(mut acc: Filter, mod_arg: FilterArg) -> Filter {
+    fn with_arg(mut self, mod_arg: FilterArg) -> Filter {
        match mod_arg {
            FilterArg::IdList(mut id_list) => {
-                // If any IDs are specified, then the filter's universe
-                // is those IDs.  If there are already IDs, append to the
-                // list.
-                if let Universe::IdList(ref mut existing) = acc.universe {
+                // If there is already an IdList condition, concatenate this one
+                // to it.  Thus multiple IdList command-line args represent an OR
+                // operation.  This assumes that the filter is still being built
+                // from command-line arguments and thus has at most one IdList
+                // condition.
+                if let Some(Condition::IdList(existing)) = self
+                    .conditions
+                    .iter_mut()
+                    .find(|c| matches!(c, Condition::IdList(_)))
+                {
                    existing.append(&mut id_list);
                } else {
-                    acc.universe = Universe::IdList(id_list);
+                    self.conditions.push(Condition::IdList(id_list));
                }
            }
            FilterArg::Condition(cond) => {
-                acc.conditions.push(cond);
+                self.conditions.push(cond);
            }
        }
-        acc
+        self
    }

+    /// combine this filter with another filter in an AND operation
+    pub(crate) fn intersect(mut self, mut other: Filter) -> Filter {
+        // simply concatenate the conditions
+        self.conditions.append(&mut other.conditions);
+
+        self
+    }
+
+    // parsers
+
    fn id_list(input: ArgList) -> IResult<ArgList, FilterArg> {
        fn to_filterarg(input: Vec<TaskId>) -> Result<FilterArg, ()> {
            Ok(FilterArg::IdList(input))
@@ -112,6 +108,8 @@ impl Filter {
        map_res(arg_matching(minus_tag), to_filterarg)(input)
    }

+    // usage
+
    pub(super) fn get_usage(u: &mut usage::Usage) {
        u.filters.push(usage::Filter {
            syntax: "TASKID[,TASKID,..]",
@@ -160,8 +158,7 @@ mod test {
        assert_eq!(
            filter,
            Filter {
-                universe: Universe::IdList(vec![TaskId::WorkingSetId(1)]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(1)])],
            }
        );
    }
@@ -173,12 +170,11 @@ mod test {
        assert_eq!(
            filter,
            Filter {
-                universe: Universe::IdList(vec![
+                conditions: vec![Condition::IdList(vec![
                    TaskId::WorkingSetId(1),
                    TaskId::WorkingSetId(2),
                    TaskId::WorkingSetId(3),
-                ]),
-                ..Default::default()
+                ])],
            }
        );
    }
@@ -190,13 +186,12 @@ mod test {
        assert_eq!(
            filter,
            Filter {
-                universe: Universe::IdList(vec![
+                conditions: vec![Condition::IdList(vec![
                    TaskId::WorkingSetId(1),
                    TaskId::WorkingSetId(2),
                    TaskId::WorkingSetId(3),
                    TaskId::WorkingSetId(4),
-                ]),
-                ..Default::default()
+                ])],
            }
        );
    }
@@ -208,11 +203,10 @@ mod test {
        assert_eq!(
            filter,
            Filter {
-                universe: Universe::IdList(vec![
+                conditions: vec![Condition::IdList(vec![
                    TaskId::WorkingSetId(1),
                    TaskId::PartialUuid(s!("abcd1234")),
-                ]),
-                ..Default::default()
+                ])],
            }
        );
    }
@@ -224,12 +218,66 @@ mod test {
        assert_eq!(
            filter,
            Filter {
-                universe: Universe::IdList(vec![TaskId::WorkingSetId(1),]),
                conditions: vec![
+                    Condition::IdList(vec![TaskId::WorkingSetId(1),]),
                    Condition::HasTag("yes".into()),
                    Condition::NoTag("no".into()),
                ],
-                ..Default::default()
+            }
+        );
+    }
+
+    #[test]
+    fn intersect_idlist_idlist() {
+        let left = Filter::parse(argv!["1,2", "+yes"]).unwrap().1;
+        let right = Filter::parse(argv!["2,3", "+no"]).unwrap().1;
+        let both = left.intersect(right);
+        assert_eq!(
+            both,
+            Filter {
+                conditions: vec![
+                    // from first filter
+                    Condition::IdList(vec![TaskId::WorkingSetId(1), TaskId::WorkingSetId(2),]),
+                    Condition::HasTag("yes".into()),
+                    // from second filter
+                    Condition::IdList(vec![TaskId::WorkingSetId(2), TaskId::WorkingSetId(3)]),
+                    Condition::HasTag("no".into()),
+                ],
+            }
+        );
+    }
+
+    #[test]
+    fn intersect_idlist_alltasks() {
+        let left = Filter::parse(argv!["1,2", "+yes"]).unwrap().1;
+        let right = Filter::parse(argv!["+no"]).unwrap().1;
+        let both = left.intersect(right);
+        assert_eq!(
+            both,
+            Filter {
+                conditions: vec![
+                    // from first filter
+                    Condition::IdList(vec![TaskId::WorkingSetId(1), TaskId::WorkingSetId(2),]),
+                    Condition::HasTag("yes".into()),
+                    // from second filter
+                    Condition::HasTag("no".into()),
+                ],
+            }
+        );
+    }
+
+    #[test]
+    fn intersect_alltasks_alltasks() {
+        let left = Filter::parse(argv!["+yes"]).unwrap().1;
+        let right = Filter::parse(argv!["+no"]).unwrap().1;
+        let both = left.intersect(right);
+        assert_eq!(
+            both,
+            Filter {
+                conditions: vec![
+                    Condition::HasTag("yes".into()),
+                    Condition::HasTag("no".into()),
+                ],
            }
        );
    }
--- a/cli/src/argparse/mod.rs
+++ b/cli/src/argparse/mod.rs
@@ -19,7 +19,7 @@ mod subcommand;

 pub(crate) use args::TaskId;
 pub(crate) use command::Command;
-pub(crate) use filter::{Condition, Filter, Universe};
+pub(crate) use filter::{Condition, Filter};
 pub(crate) use modification::{DescriptionMod, Modification};
 pub(crate) use subcommand::Subcommand;

--- a/cli/src/argparse/subcommand.rs
+++ b/cli/src/argparse/subcommand.rs
@@ -383,7 +383,7 @@ impl Sync {
 #[cfg(test)]
 mod test {
    use super::*;
-    use crate::argparse::Universe;
+    use crate::argparse::Condition;

    const EMPTY: Vec<&str> = vec![];

@@ -459,8 +459,7 @@ mod test {
    fn test_modify_description_multi() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                description: DescriptionMod::Set(s!("foo bar")),
@@ -477,8 +476,7 @@ mod test {
    fn test_append() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                description: DescriptionMod::Append(s!("foo bar")),
@@ -495,8 +493,7 @@ mod test {
    fn test_prepend() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                description: DescriptionMod::Prepend(s!("foo bar")),
@@ -513,8 +510,7 @@ mod test {
    fn test_done() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                status: Some(Status::Completed),
@@ -531,8 +527,7 @@ mod test {
    fn test_done_modify() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                description: DescriptionMod::Set(s!("now-finished")),
@@ -550,8 +545,7 @@ mod test {
    fn test_start() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                active: Some(true),
@@ -568,8 +562,7 @@ mod test {
    fn test_start_modify() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                active: Some(true),
@@ -587,8 +580,7 @@ mod test {
    fn test_stop() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                active: Some(false),
@@ -605,8 +597,7 @@ mod test {
    fn test_stop_modify() {
        let subcommand = Subcommand::Modify {
            filter: Filter {
-                universe: Universe::for_ids(vec![123]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(123)])],
            },
            modification: Modification {
                description: DescriptionMod::Set(s!("mod")),
@@ -636,8 +627,10 @@ mod test {
    fn test_report_filter_before() {
        let subcommand = Subcommand::Report {
            filter: Filter {
-                universe: Universe::for_ids(vec![12, 13]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![
+                    TaskId::WorkingSetId(12),
+                    TaskId::WorkingSetId(13),
+                ])],
            },
            report_name: "foo".to_owned(),
        };
@@ -651,8 +644,10 @@ mod test {
    fn test_report_filter_after() {
        let subcommand = Subcommand::Report {
            filter: Filter {
-                universe: Universe::for_ids(vec![12, 13]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![
+                    TaskId::WorkingSetId(12),
+                    TaskId::WorkingSetId(13),
+                ])],
            },
            report_name: "foo".to_owned(),
        };
@@ -666,8 +661,10 @@ mod test {
    fn test_report_filter_next() {
        let subcommand = Subcommand::Report {
            filter: Filter {
-                universe: Universe::for_ids(vec![12, 13]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![
+                    TaskId::WorkingSetId(12),
+                    TaskId::WorkingSetId(13),
+                ])],
            },
            report_name: "next".to_owned(),
        };
@@ -696,8 +693,10 @@ mod test {
        let subcommand = Subcommand::Info {
            debug: false,
            filter: Filter {
-                universe: Universe::for_ids(vec![12, 13]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![
+                    TaskId::WorkingSetId(12),
+                    TaskId::WorkingSetId(13),
+                ])],
            },
        };
        assert_eq!(
@@ -711,8 +710,7 @@ mod test {
        let subcommand = Subcommand::Info {
            debug: true,
            filter: Filter {
-                universe: Universe::for_ids(vec![12]),
-                ..Default::default()
+                conditions: vec![Condition::IdList(vec![TaskId::WorkingSetId(12)])],
            },
        };
        assert_eq!(