clj-commons / potemkin

some ideas which are almost good
572 stars 53 forks source link

Fix walk function for record type #54

Closed andrewboltachev closed 6 years ago

andrewboltachev commented 6 years ago

There's a problem when walk function is run on a record type, like this:

user=> (require '[potemkin.walk])
nil
user=> (defrecord R1 [a b c])
user.R1
user=> (potemkin.walk/prewalk identity [(R1. 1 2 3)])

UnsupportedOperationException Can't create empty: user.R1  user.R1 (form-init2719416585225395373.clj:1)

This happens, 'cause https://github.com/ztellman/potemkin/blob/2473a2bf056f06b03de2e47e4708f28d8a445f38/src/potemkin/walk.clj#L12 the (coll? form) check for record returns true, and it in order tries to compose an empty instance of it ((empty form)).

The following is solved in standard walk: https://github.com/clojure/clojure/blob/2224dbad5534ff23d3653b07d9dc0a60ba076dd7/src/clj/clojure/walk.clj#L47

So, in this PR I've copied this line to make it work:

user=> (require '[potemkin.walk])
nil
user=> (defrecord R1 [a b c])
user.R1
user=> (potemkin.walk/prewalk identity [(R1. 1 2 3)])
[#user.R1{:a 1, :b 2, :c 3}]
user=> 
ztellman commented 6 years ago

Thanks, I hadn't realized the implementation of clojure.walk had been updated.

andrewboltachev commented 6 years ago

N.B.: I've had a problem with versions (left it 0.4.4 and had no effect after lein install). I think version should be changed to like 0.4.5-SNAPSHOT or sth.

ztellman commented 6 years ago

Hmm, not sure why lein install wouldn't work, but I've bumped the version, let me know if the issue persists.

andrewboltachev commented 6 years ago

@ztellman works well now with 0.4.5-SNAPSHOT

ztellman commented 6 years ago

Okay, I've cut 0.4.5-alpha1.