clojure.walk should support records

Description

Problem: clojure.walk throws exceptions if used on records.

Current Patch: 0001-CLJ-1105-Support-records-in-clojure.walk.patch adds a special case for records.

See also: "faster, more flexible dispatch for clojure.walk" which could supersede this ticket.

Screened by: Alex Miller - I think this will likely be superseded by in the future, but this is a reasonable short-term step.

Environment

None

Activity

Show:
Andy Fingerhut
February 14, 2014, 7:54 PM

This comment would probably get more visibility on CLJ-1239, given that this one is closed and is still open.

chouser
February 14, 2014, 7:50 PM

The way this patch behaves can be surprising compared to regular maps:

(clojure.walk/prewalk-replace {[:a 1] nil} {:a 1, :b 2})
;=> {:b 2}

(defrecord Foo [a b])
(clojure.walk/prewalk-replace {[:a 1] nil} (map->Foo {:a 1, :b 2}))
;=> #user.Foo{:a 1, :b 2}

Note how the [:a 1] entry is removed from the map, but not from the record.

I believe also demonstrates this behavior.

Nicola Mometto
July 29, 2013, 6:45 PM

clojure.walk is not required during bootstrapping, so I don't see why we couldn't just substitute clojure.walk with clojure.walk2 given that it only changes the internal implementation to a faster one

Stuart Sierra
July 29, 2013, 6:15 PM

Attachment 0001-CLJ-1105-Support-records-in-clojure.walk.patch is a minimal patch adding support for records without altering the design of clojure.walk.

Alex Miller
July 26, 2013, 4:18 PM

Update title and adjust description slightly to focus this ticket on being an enhancement to make clojure.walk work with records.

Completed

Assignee

Unassigned

Reporter

import

Labels

Approval

Ok

Patch

Code and Test

Fix versions

Affects versions

Priority

Minor