Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changed? not propagated properly #7

Open
noprompt opened this issue Sep 22, 2014 · 1 comment
Open

changed? not propagated properly #7

noprompt opened this issue Sep 22, 2014 · 1 comment

Comments

@noprompt
Copy link
Contributor

Observe

(require '[fast-zip.core :as fz])
(require '[clojure.zip :as z])

(def xml {:content [{:content ["foo"]}]})

(-> (fz/xml-zip xml)
    (fz/append-child {:content ["bar"]})
    (fz/down)
    (.. path changed?))
;; => nil
(-> (z/xml-zip xml)
    (z/append-child {:content ["bar'"]})
    (z/down)
    (second)
    (get-in [:ppath :changed?]))
;; => true

I'm not sure the best way to fix this right away but I'm investigating it.

@noprompt
Copy link
Contributor Author

It appears the problems are with down and up.

I resolved the problem by changing down to maintain the :changed? state of the path

           (.children loc)
           (.make-node loc)
           (first cs)
-          (ZipperPath. '() (clojure.core/next cs) path (if path (conj (.pnodes path) node) [node]) nil))))))
+          (ZipperPath. '()
+                       (clojure.core/next cs)
+                       path
+                       (if path (conj (.pnodes path) node) [node])
+                       (when path (assoc path :changed? true))))))))

and for up I added the creation of a new ZipperPath in the else clause

     (.children loc)
     (.make-node loc)
     node
-    (if-let [path (.path loc)] (assoc path :changed? true))))
+    (if-let [path (.path loc)]
+      (assoc (.path loc) :changed? true)
+      (ZipperPath. '() nil nil nil true))))

The tests still pass for these changes but I'll need to update the suite to handle these cases. I will submit another pull request after #6 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant