[CLJS-700] clojure.core.reducers/fold broken Created: 27/Nov/13 Updated: 29/Nov/13 Resolved: 29/Nov/13
|Reporter:||Jonas De Vuyst||Assignee:||Unassigned|
|Attachments:||cljs_700.patch cljs-fold-fix-2.patch cljs-fold-fix.patch|
|Patch:||Code and Test|
clojure.core.reducers/fold currently does not work correctly if both a reducef and combinef are passed as arguments.
It is expected that (r/fold + + [1 2 3]) would return 6 (as do (r/fold + [1 2 3]) and (r/reduce + [1 2 3])). However, this is not the case because reducers.cljs currently defines fold as follows:
(def fold reduce)
As a result, the second + is used as the initial value for the reduce operation (instead of the value returned by ).
The attached patch fixes this bug. It also adds support for the protocol CollFold in CLJS.
(I will send in the CA tomorrow.)
|Comment by David Nolen [ 28/Nov/13 8:05 AM ]|
This mostly looks good but clojure.lang.PersistentHashMap should be cljs.core/PersistentHashMap and we should have a corresponding test.
|Comment by Jonas De Vuyst [ 29/Nov/13 1:45 PM ]|
I added a new patch, in which cljs.core/PersistentHashMap is substituted for clojure.lang.PersistentHashMap – although that part is actually commented out because CLJS maps do not have a method .fold.
I also added tests for reducing and folding of maps (which I also tested in Clojure).
|Comment by David Nolen [ 29/Nov/13 2:45 PM ]|
The patch has not been formatted so that it can be applied with git am. Please generate the patch following these instructions - http://github.com/clojure/clojurescript/wiki/Patches.
|Comment by Jonas De Vuyst [ 29/Nov/13 3:58 PM ]|
Oh, sorry. I have attached a new patch.
|Comment by David Nolen [ 29/Nov/13 4:00 PM ]|