<< Back to previous view

[CLJ-1317] clojure.zip/seq-zip returns spurious nils during traversal Created: 31/Dec/13  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: zip

Attachments: Text File 0001-CLJ-1317-fix-seq-zip-to-avoid-spurious-nils.patch    
Patch: Code

 Description   

Problem reported by Lee Spector on the mailing list:

https://groups.google.com/d/msg/clojure/8TL7IGmE7N0/u1xfgTOLDRgJ

Here's a quote from Lee's post describing the problem:

Here's an illustration, stepping through '(() 0) with next and printing the node at each step: 

(loop [z (zip/seq-zip '(() 0))] 
  (if (zip/end? z) 
    :done 
    (do (println (zip/node z)) 
      (recur (zip/next z))))) 

That produces: 

(() 0) 
() 
nil 
0 
:done 

I don't expect the nil to be there. 

The underlying cause is that seq-zip passes identity as the children argument to zipper. Applied to (), this returns (), which is truthy, leading zipper to descend into a non-existent subtree.

One natural solution would be to use seq in place of identity:

(defn seq-zip [root]
  (zipper seq?
          seq  ;; changed
          (fn [node children] (with-meta children (meta node)))
          root))

With this change, no nil is produced in the example above. Patch with this change forthcoming.



 Comments   
Comment by Michał Marczyk [ 31/Dec/13 5:52 PM ]

Note that the docstring of clojure.zip/zipper asks that the children argument return a seq of children. The rest of clojure.zip, however, expects nil to be returned when there are no children, as evidenced by this problem.

One could argue that this behaviour of the rest of clojure.zip should be fixed, but I think it makes sense and is convenient. Perhaps the docstring should be adjusted, though.





[CLJ-1097] node-seq for clojure.zip Created: 29/Oct/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: zip

Attachments: File node-seq.diff    
Patch: Code and Test

 Description   

Many times it's easier to get to a zipper node via (first (filter pred ...)) instead of manually walking the tree via next/up/down. Other times it's easier to process certain nodes via filter & map instead of again, walking the tree. This patch provides a single function called node-seq that uses zip/next to generate a lazy-seq of nodes. Tests provide two examples.






[CLJ-941] NullPointerException possible with seq-zip Created: 26/Feb/12  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.3
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: zip


 Description   

For example:

Clojure 1.3.0
user=> (require '[clojure.zip :as z])
nil
user=> (-> (z/seq-zip (list 1)) z/down z/remove)
NullPointerException clojure.core/with-meta (core.clj:211)

Possibly the make-node function for seq-zip should be:

(fn [node children] (with-meta (or children ()) (meta node)))



 Comments   
Comment by Greg Chapman [ 26/Feb/12 5:54 PM ]

Also the docstring for zipper should probably be updated to indicate that the children parameter can be nil.





Generated at Thu Nov 27 21:12:25 CST 2014 using JIRA 4.4#649-r158309.