ClojureScript

Bad Exception message when multimethod has no dispatch-fn

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

When a multi-fn has no dispatch method for a given value the current exception string prints the cljs.core/name function instead of the actual name of the mf. Minor bug but makes it kinda hard to track down which multi-fn actually failed to dispatch.

The attached patch fixes that but directly accessing the name property of the multi-fn which is not very clean but better than the current error. AFAICT cljs doesnt have the clojure.lang.Named protocol, which would probably be cleaner.

Activity

Thomas Heller made changes -
Field Original Value New Value
Attachment cljs-multifn-ex-message.patch [ 11847 ]
Hide
Thomas Heller added a comment -

Corrected the .patch

Show
Thomas Heller added a comment - Corrected the .patch
Thomas Heller made changes -
Attachment cljs-multifn-ex-message.patch [ 11848 ]
Hide
David Nolen added a comment -

ClojureScript now has an INamed protocol

Show
David Nolen added a comment - ClojureScript now has an INamed protocol
Hide
Thomas Heller added a comment -

Updated the .patch to implement INamed for cljs.core/MultiFn, turning its name into a real symbol.

Tests pass but I dont know if that part of the code is actually tested.

Show
Thomas Heller added a comment - Updated the .patch to implement INamed for cljs.core/MultiFn, turning its name into a real symbol. Tests pass but I dont know if that part of the code is actually tested.
Thomas Heller made changes -
Attachment cljs-multi-fn-ex-msg-inamed.patch [ 11865 ]
Hide
Thomas Heller added a comment -

Oh, I'm not quite sure that the way I resolved the namespace for the symbol is totally correct. It works but I had to dig a bit.

Show
Thomas Heller added a comment - Oh, I'm not quite sure that the way I resolved the namespace for the symbol is totally correct. It works but I had to dig a bit.
Thomas Heller made changes -
Attachment cljs-multifn-ex-message.patch [ 11848 ]
David Nolen made changes -
Priority Minor [ 4 ] Major [ 3 ]
Hide
David Nolen added a comment -

Rebased patch welcome, thanks!

Show
David Nolen added a comment - Rebased patch welcome, thanks!
Hide
Thomas Heller added a comment -

Uh, forgot about this. The important part of this issue was fixed a while back, the exception no long contains the source code to the cljs.core/name function.

The name however still is not fully qualified, updated the patch to only contain the INamed implementation for cljs.core/MultiFn so it will correctly report its full name.

Show
Thomas Heller added a comment - Uh, forgot about this. The important part of this issue was fixed a while back, the exception no long contains the source code to the cljs.core/name function. The name however still is not fully qualified, updated the patch to only contain the INamed implementation for cljs.core/MultiFn so it will correctly report its full name.
Thomas Heller made changes -
Attachment multi-fn-inamed.diff [ 13558 ]
Thomas Heller made changes -
Attachment cljs-multi-fn-ex-msg-inamed.patch [ 11865 ]
David Nolen made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: