Fix :exclude[] implementation and documentation

It was not clear before that the excluded paths are the output
of the filter rather than the matching input.

Author: christian-schilling

docs/src/reference/filters.md

@@ -45,7 +45,9 @@ Inside of a composition ``x=:filter`` can be used as an alternative spelling for
 ``:filter:prefix=x``.
 
 ### Exclusion **`:exclude[:filter]`**
-Remove all paths matching ``:filter`` from the input tree.
+Remove all paths present in the *output* of ``:filter`` from the input tree.
+It should generally be avoided to use any filters that change paths and instead only
+use filters that select paths without altering them.
 
 ### Workspace **`:workspace=a`**
 Similar to ``:/a`` but also looks for a ``workspace.josh`` file inside the

src/cache.rs

@@ -1,7 +1,7 @@
 use super::*;
 use std::collections::HashMap;
 
-const VERSION: u64 = 6;
+const VERSION: u64 = 8;
 
 lazy_static! {
     static ref DB: std::sync::Mutex<Option<sled::Db>> = std::sync::Mutex::new(None);

src/filter/mod.rs

@@ -73,6 +73,7 @@ enum Op {
     Compose(Vec<Filter>),
     Chain(Filter, Filter),
     Subtract(Filter, Filter),
+    Exclude(Filter),
 }
 
 /// Pretty print the filter on multiple lines with initial indentation level.
@@ -113,10 +114,10 @@ fn pretty2(op: &Op, indent: usize, compose: bool) -> String {
     };
     match op {
         Op::Compose(filters) => ff(filters, "", indent),
-        Op::Subtract(af, bf) => match (to_op(*af), to_op(*bf)) {
-            (Op::Nop, Op::Compose(filters)) => ff(&filters, "exclude", indent),
-            (Op::Nop, b) => format!(":exclude[{}]", pretty2(&b, indent, false)),
-            _ => ff(&vec![*af, *bf], "subtract", indent + 4),
+        Op::Subtract(af, bf) => ff(&vec![*af, *bf], "subtract", indent + 4),
+        Op::Exclude(bf) => match to_op(*bf) {
+            Op::Compose(filters) => ff(&filters, "exclude", indent),
+            b => format!(":exclude[{}]", pretty2(&b, indent, false)),
         },
         Op::Chain(a, b) => match (to_op(*a), to_op(*b)) {
             (Op::Subdir(p1), Op::Prefix(p2)) if p1 == p2 => {
@@ -157,6 +158,9 @@ fn spec2(op: &Op) -> String {
         Op::Subtract(a, b) => {
             format!(":subtract[{},{}]", spec(*a), spec(*b))
         }
+        Op::Exclude(b) => {
+            format!(":exclude[{}]", spec(*b))
+        }
         Op::Workspace(path) => {
             format!(":workspace={}", path.to_string_lossy())
         }
@@ -389,9 +393,21 @@ fn apply_to_commit2(
             };
             let bf = repo.find_tree(bf)?;
             let bu = unapply(transaction, *b, bf, tree::empty(repo))?;
-            let ba = apply(transaction, *a, bu)?;
-
-            repo.find_tree(tree::subtract(repo, af, ba.id())?)?
+            let ba = apply(transaction, *a, bu)?.id();
+            repo.find_tree(tree::subtract(repo, af, ba)?)?
+        }
+        Op::Exclude(b) => {
+            let bf = {
+                transaction
+                    .repo()
+                    .find_commit(some_or!(
+                        apply_to_commit2(&to_op(*b), commit, transaction)?,
+                        { return Ok(None) }
+                    ))
+                    .map(|x| x.tree_id())
+                    .unwrap_or(tree::empty_id())
+            };
+            repo.find_tree(tree::subtract(repo, commit.tree_id(), bf)?)?
         }
         _ => apply(transaction, filter, commit.tree()?)?,
     };
@@ -476,8 +492,12 @@ fn apply2<'a>(
             let af = apply(transaction, *a, tree.clone())?;
             let bf = apply(transaction, *b, tree.clone())?;
             let bu = unapply(transaction, *b, bf, tree::empty(repo))?;
-            let ba = apply(transaction, *a, bu)?;
-            Ok(repo.find_tree(tree::subtract(repo, af.id(), ba.id())?)?)
+            let ba = apply(transaction, *a, bu)?.id();
+            Ok(repo.find_tree(tree::subtract(repo, af.id(), ba)?)?)
+        }
+        Op::Exclude(b) => {
+            let bf = apply(transaction, *b, tree.clone())?.id();
+            Ok(repo.find_tree(tree::subtract(repo, tree.id(), bf)?)?)
         }
 
         Op::Paths => tree::pathstree("", tree.id(), transaction),
@@ -640,21 +660,19 @@ fn unapply2<'a>(
             }
         }
 
-        Op::Subtract(a, b) => match (to_op(*a), to_op(*b)) {
-            (Op::Nop, b) => {
-                let subtracted = tree::subtract(
-                    transaction.repo(),
-                    tree.id(),
-                    unapply2(transaction, &b, tree, tree::empty(transaction.repo()))?.id(),
-                )?;
-                Ok(transaction.repo().find_tree(tree::overlay(
-                    transaction.repo(),
-                    parent_tree.id(),
-                    subtracted,
-                )?)?)
-            }
-            _ => return Err(josh_error("filter not reversible")),
-        },
+        Op::Subtract(_, _) => return Err(josh_error("filter not reversible")),
+        Op::Exclude(b) => {
+            let subtracted = tree::subtract(
+                transaction.repo(),
+                tree.id(),
+                unapply(transaction, *b, tree, tree::empty(transaction.repo()))?.id(),
+            )?;
+            Ok(transaction.repo().find_tree(tree::overlay(
+                transaction.repo(),
+                parent_tree.id(),
+                subtracted,
+            )?)?)
+        }
         Op::Glob(pattern) => {
             let pattern = glob::Pattern::new(pattern)?;
             let options = glob::MatchOptions {

src/filter/opt.rs

@@ -75,6 +75,7 @@ pub fn simplify(filter: Filter) -> Filter {
         Op::Subtract(a, b) => match (to_op(a), to_op(b)) {
             (a, b) => Op::Subtract(simplify(to_filter(a)), simplify(to_filter(b))),
         },
+        Op::Exclude(b) => Op::Exclude(simplify(b)),
         _ => to_op(filter),
     });
 
@@ -128,6 +129,7 @@ pub fn flatten(filter: Filter) -> Filter {
         Op::Subtract(a, b) => match (to_op(a), to_op(b)) {
             (a, b) => Op::Subtract(flatten(to_filter(a)), flatten(to_filter(b))),
         },
+        Op::Exclude(b) => Op::Exclude(flatten(b)),
         _ => to_op(filter),
     });
 
@@ -339,6 +341,9 @@ fn step(filter: Filter) -> Filter {
             (_, Op::Empty) => Op::Empty,
             (a, b) => Op::Chain(step(to_filter(a)), step(to_filter(b))),
         },
+        Op::Exclude(b) if b == to_filter(Op::Nop) => Op::Empty,
+        Op::Exclude(b) if b == to_filter(Op::Empty) => Op::Nop,
+        Op::Exclude(b) => Op::Exclude(step(b)),
         Op::Subtract(a, b) if a == b => Op::Empty,
         Op::Subtract(af, bf) => match (to_op(af), to_op(bf)) {
             (Op::Empty, _) => Op::Empty,

src/filter/parse.rs

@@ -53,9 +53,7 @@ fn parse_item(pair: pest::iterators::Pair<Rule>) -> JoshResult<Op> {
                 [cmd, args] => {
                     let g = parse_group(args)?;
                     match *cmd {
-                        "exclude" => {
-                            Ok(Op::Subtract(to_filter(Op::Nop), to_filter(Op::Compose(g))))
-                        }
+                        "exclude" => Ok(Op::Exclude(to_filter(Op::Compose(g)))),
                         "subtract" if g.len() == 2 => Ok(Op::Subtract(g[0], g[1])),
                         _ => Err(josh_error("parse_item: no match")),
                     }

tests/filter/empty_orphan.t

@@ -100,9 +100,10 @@ Empty root commits from unrelated parts of the tree should not be included
   c/file2
   c/file3
 
-  $ josh-filter -s c=:exclude[:/sub1] master
+  $ josh-filter -s c=:exclude[::sub1/] master
+  [4] :prefix=sub1
   [5] :/sub1
-  [5] :exclude[:/sub1]
+  [5] :exclude[::sub1/]
   [6] :prefix=c
 
   $ git log FILTERED_HEAD --graph --pretty=%s
@@ -116,8 +117,9 @@ Empty root commits from unrelated parts of the tree should not be included
 
   $ josh-filter -s :prefix=x FILTERED_HEAD
   [3] :prefix=x
+  [4] :prefix=sub1
   [5] :/sub1
-  [5] :exclude[:/sub1]
+  [5] :exclude[::sub1/]
   [6] :prefix=c
 
   $ git ls-tree --name-only -r FILTERED_HEAD

tests/filter/exclude_compose.t

@@ -14,59 +14,78 @@
   $ git add sub2
   $ git commit -m "add file2" 1> /dev/null
 
-  $ josh-filter -s :exclude[:/sub2] master --update refs/heads/hidden
-  [1] :exclude[:/sub2]
+  $ mkdir sub3
+  $ echo contents1 > sub3/file1
+  $ git add sub3
+  $ git commit -m "add file3" 1> /dev/null
+
+  $ josh-filter -s :exclude[::sub2/] master --update refs/heads/hidden
+  [1] :prefix=sub2
   [2] :/sub2
-  $ git checkout hidden 1> /dev/null
-  Switched to branch 'hidden'
+  [2] :exclude[::sub2/]
+  $ git checkout -q hidden 1> /dev/null
   $ tree
   .
-  `-- sub1
+  |-- sub1
+  |   `-- file1
+  `-- sub3
       `-- file1
 
-  1 directory, 1 file
+  2 directories, 2 files
   $ git log --graph --pretty=%s
+  * add file3
   * add file1
 
   $ echo contents3 > sub1/file3
   $ git add sub1/file3
   $ git commit -m "add sub1/file3" 1> /dev/null
 
-  $ josh-filter -s :exclude[:/sub1,:/sub2] master --update refs/josh/filtered
+  $ josh-filter -s :exclude[::sub1/,::sub2/] master --update refs/josh/filtered
   [1] :/sub1
-  [1] :exclude[
-      :/sub1
-      :/sub2
-  ]
-  [1] :exclude[:/sub2]
+  [1] :prefix=sub1
+  [1] :prefix=sub2
   [2] :/sub2
   [2] :[
-      :/sub1
-      :/sub2
+      ::sub1/
+      ::sub2/
   ]
+  [2] :exclude[
+      ::sub1/
+      ::sub2/
+  ]
+  [2] :exclude[::sub2/]
 
-  $ git checkout refs/josh/filtered
-  Note: switching to 'refs/josh/filtered'.
-  
-  You are in 'detached HEAD' state. You can look around, make experimental
-  changes and commit them, and you can discard any commits you make in this
-  state without impacting any branches by switching back to a branch.
-  
-  If you want to create a new branch to retain commits you create, you may
-  do so (now or later) by using -c with the switch command. Example:
-  
-    git switch -c <new-branch-name>
-  
-  Or undo this operation with:
-  
-    git switch -
-  
-  Turn off this advice by setting config variable advice.detachedHead to false
-  
-  HEAD is now at bb282e9 add file1
+  $ git checkout -q refs/josh/filtered
   $ tree
   .
-  `-- sub1
+  `-- sub3
       `-- file1
 
   1 directory, 1 file
+
+  $ josh-filter -s :exclude[sub1=:/sub3] master --update refs/josh/filtered
+  [1] :/sub1
+  [1] :prefix=sub2
+  [2] :/sub2
+  [2] :/sub3
+  [2] :[
+      ::sub1/
+      ::sub2/
+  ]
+  [2] :exclude[
+      ::sub1/
+      ::sub2/
+  ]
+  [2] :exclude[::sub2/]
+  [2] :prefix=sub1
+  [3] :exclude[:/sub3:prefix=sub1]
+
+  $ git checkout -q refs/josh/filtered
+  $ tree
+  .
+  |-- sub2
+  |   `-- file2
+  `-- sub3
+      `-- file1
+  
+  2 directories, 2 files

tests/filter/hide_view.t

@@ -33,10 +33,11 @@
 
   2 directories, 3 files
 
-  $ josh-filter -s c=:exclude[:/sub1] master --update refs/josh/filter/master
+  $ josh-filter -s c=:exclude[::sub1/] master --update refs/josh/filter/master
   [1] :prefix=c
   [2] :/sub1
-  [2] :exclude[:/sub1]
+  [2] :exclude[::sub1/]
+  [2] :prefix=sub1
   $ git checkout josh/filter/master 2> /dev/null
   $ git log --graph --pretty=%s
   * add file3
@@ -51,8 +52,9 @@
   $ josh-filter -s c=:exclude[::sub1/file2] master --update refs/josh/filter/master
   [2] :/sub1
   [2] ::sub1/file2
-  [2] :exclude[:/sub1]
+  [2] :exclude[::sub1/]
   [2] :exclude[::sub1/file2]
+  [2] :prefix=sub1
   [3] :prefix=c
   $ git checkout josh/filter/master 2> /dev/null
   $ git log --graph --pretty=%s
@@ -72,9 +74,10 @@
   [2] :/sub1
   [2] ::sub1/file2
   [2] ::sub2/file3
-  [2] :exclude[:/sub1]
+  [2] :exclude[::sub1/]
   [2] :exclude[::sub1/file2]
   [2] :exclude[::sub2/file3]
+  [2] :prefix=sub1
   [4] :prefix=c
   $ git checkout josh/filter/master 2> /dev/null
   $ git log --graph --pretty=%s