Fix non-recognized recur inside case in the debugger.
#663
Conversation
Case expands the non-default branches into a map. This can be seen
when evaluating the following code:
`(walk/macroexpand-all '(case i
1 "hello"
"world"))`
The `contains-recur?` did so far not take that into account.
The previous `contains-recur?` did not handle `case` when a map return
value was given correctly. Example:
(case i
nil {:foo :bar}
:fuss)
This commit solve this.
| ;; this depends on internal logic | ||
| (map? form) (some contains-recur? (vals form)) | ||
| (vector? form) (some contains-recur? (seq form)) | ||
| :else false)) |
There was a problem hiding this comment.
I don't know if this is the best way to handle the issue. I am open for other suggestions. I mean normally a recur should only appear in a map when used inside a case.
There was a problem hiding this comment.
I think your approach is pretty reasonable, but perhaps @Malabarba or @vspinu have some better ideas.
|
The code looks good to me overall, we just need a changelog entry. P.S. I really appreciate that you took the time to get familiar with the debugger and tackle some issues there! 🙇 |
|
@bbatsov I don't know if you saw it, but I also left a longer comment in clojure-emacs/cider#2772 outlining some more general problem in the debugger. No pressure to reply soon. Just wanted to make sure you had seen it. I can also open an issue here for discussion as it is more related to cider-nrepl than to the cider emacs side. |
|
Just replied there as well. |
Case expands the non-default branches into a map. This can be seen
when evaluating the following code:
The
contains-recur?function did so far not take that into account.Before submitting a PR make sure the following things have been done:
I was able to run tests for 1.9 and 1.10. 1.9 had 1 failure non related to my code and 1.10 none.
I was unable to run the tests for 1.8 because of some leiningen issue.
This should be fixing cider#2772