MapEntry type crashes diff

[ingesolvoll]

EDIT: This happens in CLJS in the browser.

Case: You use clojure.core/find to produce a vector-like structure:

(def x {:a "a"
        :b "b"})

(find x :a) ;;  => [:a "a"]
;; The above is an instance of `MapEntry`

Map entries can mostly be used as a seq. But if you try to diff a structure that includes map entries, editscript will match them as associatives (vectors) and try to kv reduce them. That throws an exception.

[ingesolvoll]

Reproduction:

(editscript.core/diff nil (first {:a :b}))

[huahaiy]

Thanks. I will look into it.

[huahaiy]

OK, we can make a MapEntry a :lst instead, that will remove the exception.

However, keep in mind that we should not expect to retain the MapEntry type.

For example,

(def d (diff (first {:a :c}) (first {:a :b})) => [[[1] :r :b]], and (patch (first {:a :c}) d) => (:a :b)

However, the Clojure value equality ensures that (= (first {:a :b}) (patch (first {:a :c}) d)), so it is sufficient for the purpose for which Editscript is designed.

[lnostdal]

Is this fix a good idea? If a MapEntry is no longer a MapEntry after patching, can't that break things in surprising and now silent(!) ways? It should probably be outside the scope of ES to serialize/deserialize a rich set of data types, so perhaps MapEntry should be left as is [replaced] or if not be serialized/deserialized proper by ES.

edit: having MapEntry part of the core types handled by ES seems OK to me, but I'm not 100% sure. ......actually no; it seems like this small thing should just be a value, and just passed and replaced as is. A serialization library should then deal with this particular value.

(let [a (first {:a 42})
      b (first {:a 21})
      new-a (->> (editscript.core/diff a b)
                 (editscript.core/patch a))]
  (assert (instance? clojure.lang.MapEntry a))
  (assert (instance? clojure.lang.MapEntry b))
  (assert (instance? clojure.lang.MapEntry new-a))) ;; !!

[lnostdal]

This seems to work correctly here: https://github.com/lnostdal/editscript/commit/2bc883cd8c24e72c2452010bf1b5deb8fbc39a77

MapEntries are still MapEntries, and there's no exception from kv reduce.

(let [a (first {:a 42})
      b (first {:a 21})
      new-a (->> (editscript.core/diff a b)
                 (editscript.core/patch a))]
  (assert (instance? clojure.lang.MapEntry a))
  (assert (instance? clojure.lang.MapEntry b))
  (assert (instance? clojure.lang.MapEntry new-a)) ;; now succeeds
  {:a a, :b b, :new-a new-a})

{:a [:a 42], :b [:a 21], :new-a [:a 21]}

[huahaiy]

Good idea. Since the shape of MapEntry is fixed, replacing it as a whole is not going to create a lot of overhead. I will take this idea. Do you mind sending a PR?