Fix inconsistent trees from getindex and merge.

[DrChainsaw]

Fixes #39

Testing revealed that the root issue is that getindex for both globs and regexps returns a tree where the children of the root does not have the root as parent. Same thing also happens after merge.

Not sure if this is intentional, but it seems to me that a tree like this is inconsistent and should be considered invalid. Another option could be to add a call to setparent in the convenience “copy” constructor for FileTree if one wants to be really strict about not allowing such trees.

It turned out that mv and cp (or in fact regex_rewrite_tree and attach) relied on this behavior to leave the root out of all processing. I first tried to change attach to not add the root node (i.e last line changed to merge(t, t1; combine=combine) and assume that path argument included the path to the root.

This did however cause problems when root node is a subdirectory as it would change the tree to use the root of the root node as, uhm, root (i.e if root node has path a/b then the returned tree would have a as root and b as its child). This also had the adverse consequence that the public API for mv and cp would break as regexes where now matched against the whole path including the root node. Both these problems where caught by the mv tests in taxi.jl.

Instead I resorted to just truncating the canonical path from behind in regex_rewrite_tree. Not sure if this is the most elegant way to do it.

[codecov-io]

Codecov Report

Merging #40 (6e1ee98) into master (43d7baa) will increase coverage by 2.82%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   84.22%   87.05%   +2.82%     
==========================================
  Files           6        6              
  Lines         317      363      +46     
==========================================
+ Hits          267      316      +49     
+ Misses         50       47       -3     

Impacted Files	Coverage Δ
src/patterns.jl	84.09% <100.00%> (+8.41%)	:arrow_up:
src/tree-ops.jl	83.17% <100.00%> (+2.12%)	:arrow_up:
src/values.jl	97.61% <0.00%> (+0.32%)	:arrow_up:
src/parallelism.jl	81.48% <0.00%> (+0.71%)	:arrow_up:
src/datastructure.jl	88.73% <0.00%> (+2.78%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 43d7baa...6e1ee98. Read the comment docs.

[DrChainsaw]

@shashi Bump?

In case you have more pressing matters on your table or just don't want to context switch: I can work around the issue from the outside by just adding |> setparent after any getindex/mv/merge so this is just a bit of an inconvenience for me atm.

[shashi]

Sorry about the delay here. This got drowned out in the other notifications. Thanks for pinging again! This looks good to me, except for that comment I left.

[DrChainsaw]

Thanks for checking!

Do you have an opinion on this:

Another option could be to add a call to setparent in the convenience “copy” constructor for FileTree if one wants to be really strict about not allowing such trees.

Is there some reason one should be able to create trees where nodes are not the parent of their children?