diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2166853f..5bff8cb7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,9 @@
* Bump `cljfmt` to 0.9.2.
* Bump `puget` to 1.3.4.
+* [#769](https://github.com/clojure-emacs/cider-nrepl/issues/769): Introduce new `#break!` and `#dbg!` reader macros for breaking on exception
+* [#772](https://github.com/clojure-emacs/cider-nrepl/issues/772): Improve debugger heuristics on "interesting" breakpoints (Always break on forms tagged with #break, including vectors and maps)
+* The `wrap-debug` middleware now adds an optional `:caught-msg` key to the `eval` op response, containing the exception message when caught by the debugger.
## 0.30.0 (2023-01-31)
diff --git a/doc/modules/ROOT/pages/hacking.adoc b/doc/modules/ROOT/pages/hacking.adoc
index 30471363..b0867554 100644
--- a/doc/modules/ROOT/pages/hacking.adoc
+++ b/doc/modules/ROOT/pages/hacking.adoc
@@ -1,12 +1,12 @@
= Hacking on cider-nrepl
-Hacking on cider-nrepl requires nothing bit a bit of knowledge of Clojure and nREPL.
+Hacking on cider-nrepl requires nothing but a bit of knowledge of Clojure and nREPL.
In this section we'll take a look at some practical tips to make you more productive
while working on the project.
== Makefile
-cider-nrepl has some pretty complicated Lein profiles, as it has to deal with multiple version of
+cider-nrepl has some pretty complicated Lein profiles, as it has to deal with multiple versions of
Clojure and ClojureScript, plus dependency inlining with Mr. Anderson. That's why we've
added a good old `Makefile` to save you the trouble of having to think about the profiles
and just focus on the tasks at hand.
diff --git a/src/cider/nrepl/middleware/debug.clj b/src/cider/nrepl/middleware/debug.clj
index ad51742d..2c7605ec 100644
--- a/src/cider/nrepl/middleware/debug.clj
+++ b/src/cider/nrepl/middleware/debug.clj
@@ -284,15 +284,18 @@ this map (identified by a key), and will `dissoc` it afterwards."}
:rendered pr-str)))
(defn- debug-stacktrace
- "Create a dummy exception, send its stack."
- []
+ "Send the stacktrace of `value` if it is an exception.
+ Otherwise, create a dummy exception to view the call stack at the current location."
+ [value]
(debugger-send
{:status :stack
- :causes [{:class "StackTrace"
- :message "Harmless user-requested stacktrace"
- :stacktrace (-> (Exception. "Dummy")
- (stacktrace.analyzer/analyze (::print/print-fn *msg*))
- last :stacktrace)}]}))
+ :causes (if (instance? Throwable value)
+ (stacktrace.analyzer/analyze value (::print/print-fn *msg*))
+ [{:class "StackTrace"
+ :message "Harmless user-requested stacktrace"
+ :stacktrace (-> (Exception. "Dummy")
+ (stacktrace.analyzer/analyze (::print/print-fn *msg*))
+ last :stacktrace)}])}))
(def debug-commands
"An unsorted set of commands supported by the debugger."
@@ -365,7 +368,7 @@ this map (identified by a key), and will `dissoc` it afterwards."}
value)
:here (do (skip-breaks! :before coord (:code dbg-state) force?)
value)
- :stacktrace (do (debug-stacktrace)
+ :stacktrace (do (debug-stacktrace value)
(recur value dbg-state))
:trace (do (skip-breaks! :trace)
value)
@@ -488,6 +491,13 @@ this map (identified by a key), and will `dissoc` it afterwards."}
~@body))
(defmacro breakpoint-with-initial-debug-bindings
+ {:style/indent 1}
+ [form dbg-state original-form]
+ `(with-initial-debug-bindings
+ (breakpoint
+ ~form ~dbg-state ~original-form)))
+
+(defmacro breakpoint-if-interesting-with-initial-debug-bindings
{:style/indent 1}
[form dbg-state original-form]
`(with-initial-debug-bindings
@@ -568,48 +578,48 @@ this map (identified by a key), and will `dissoc` it afterwards."}
(and (seq? form)
(irrelevant-return-value-forms (first form)))))
+(defmacro breakpoint
+ "Wrap form in a breakpoint unconditionally."
+ [form {:keys [coor] :as dbg-state} original-form]
+ (let [condition (:break/when (meta form))]
+ (if condition
+ ;; If there is a condition and it is falsy, we need to skip
+ ;; the current level (:deeper than parent coor), but only
+ ;; once. Next time, we need to test the condition again.
+ `(let [old-breaks# @*skip-breaks*]
+ (when-not ~condition
+ (skip-breaks! :deeper ~(vec (butlast coor)) (:code (:msg ~'STATE__)) false))
+ (try
+ (expand-break ~form ~dbg-state ~original-form)
+ ;; in case :continue-all was requested in a deeper level
+ ;; we don't want go back to the old-breaks
+ (finally (when (not= :all (:mode @*skip-breaks*))
+ (reset! *skip-breaks* old-breaks#)))))
+ `(expand-break ~form ~dbg-state ~original-form))))
+
(defmacro breakpoint-if-interesting
"Wrap form in a breakpoint if it looks interesting.
Uninteresting forms are symbols that resolve to `clojure.core`
(taking locals into account), and sexps whose head is present in
`irrelevant-return-value-forms`. Used as :breakfunction in `tag-form`."
- [form {:keys [coor] :as dbg-state} original-form]
+ [form dbg-state original-form]
(if (uninteresting-form? &env form)
form
- (let [condition (:break/when (meta form))]
- (if condition
- ;; If there is a condition and it is falsy, we need to skip
- ;; the current level (:deeper than parent coor), but only
- ;; once. Next time, we need to test the condition again.
- `(let [old-breaks# @*skip-breaks*]
- (when-not ~condition
- (skip-breaks! :deeper ~(vec (butlast coor)) (:code (:msg ~'STATE__)) false))
- (try
- (expand-break ~form ~dbg-state ~original-form)
- ;; in case :continue-all was requested in a deeper level
- ;; we don't want go back to the old-breaks
- (finally (when (not= :all (:mode @*skip-breaks*))
- (reset! *skip-breaks* old-breaks#)))))
- `(expand-break ~form ~dbg-state ~original-form)))))
+ `(breakpoint ~form ~dbg-state ~original-form)))
(defmacro breakpoint-if-exception
"Wrap form in a try-catch that has breakpoint on exception.
- Ignores uninteresting forms.
- Uninteresting forms are symbols that resolve to `clojure.core`
- (taking locals into account), and sexps whose head is present in
- `irrelevant-return-value-forms`. Used as :breakfunction in `tag-form`."
+ Used as :breakfunction in `tag-form`."
[form dbg-state original-form]
- (if (uninteresting-form? &env form)
- form
- `(try ~form
- (catch Throwable ex#
- (let [exn-message# (.getMessage ex#)
- break-result# (expand-break exn-message# ~dbg-state ~original-form)]
- (if (= exn-message# break-result#)
- ;; if they continued then rethrow the exception
- (throw ex#)
- ;; otherwise return the value they injected
- break-result#))))))
+ `(try ~form
+ (catch Throwable ex#
+ (let [~'STATE__ (assoc-in ~'STATE__ [:msg :caught-msg] (.getMessage ex#))
+ break-result# (expand-break ex# ~dbg-state ~original-form)]
+ (if (= ex# break-result#)
+ ;; if they continued then rethrow the exception
+ (throw ex#)
+ ;; otherwise return the value they injected
+ break-result#)))))
;;; ## Data readers
;;
@@ -617,19 +627,19 @@ this map (identified by a key), and will `dissoc` it afterwards."}
(defn breakpoint-reader
"#break reader. Mark `form` for breakpointing."
[form]
- (ins/tag-form form #'breakpoint-with-initial-debug-bindings))
+ (ins/tag-form form #'breakpoint-with-initial-debug-bindings true))
(defn debug-reader
"#dbg reader. Mark all forms in `form` for breakpointing.
`form` itself is also marked."
[form]
(ins/tag-form (ins/tag-form-recursively form #'breakpoint-if-interesting)
- #'breakpoint-with-initial-debug-bindings))
+ #'breakpoint-if-interesting-with-initial-debug-bindings))
(defn break-on-exception-reader
"#exn reader. Wrap `form` in try-catch and break only on exception"
[form]
- (ins/tag-form form #'breakpoint-if-exception-with-initial-debug-bindings))
+ (ins/tag-form form #'breakpoint-if-exception-with-initial-debug-bindings true))
(defn debug-on-exception-reader
"#dbgexn reader. Mark all forms in `form` for breakpointing on exception.
@@ -655,6 +665,10 @@ this map (identified by a key), and will `dissoc` it afterwards."}
(eval form1)))
(throw e))))))
+(def ^:dynamic *debug-data-readers*
+ "Reader macros like #dbg which cause code to be instrumented when present."
+ '#{dbg exn dbgexn break light})
+
;;; ## Middleware
(defn- maybe-debug
"Return msg, prepared for debugging if code contains debugging macros."
@@ -666,8 +680,8 @@ this map (identified by a key), and will `dissoc` it afterwards."}
fake-reader (fn [x] (reset! has-debug? true) x)]
(binding [*ns* (find-ns (symbol (or ns "user")))
*data-readers* (->> (repeat fake-reader)
- (interleave '[dbg exn dbgexn break light])
- (apply assoc *data-readers*))]
+ (zipmap *debug-data-readers*)
+ (merge *data-readers*))]
(try
;; new-line in REPL always throws; skip for debug convenience
(when (> (count code) 3)
diff --git a/src/cider/nrepl/middleware/util/instrument.clj b/src/cider/nrepl/middleware/util/instrument.clj
index d236e941..1fb28238 100644
--- a/src/cider/nrepl/middleware/util/instrument.clj
+++ b/src/cider/nrepl/middleware/util/instrument.clj
@@ -225,9 +225,11 @@
;; Other coll types are safe, so we go inside them and only
;; instrument what's interesting.
;; Do we also need to check for seq?
- coll? (doall (instrument-coll form))
+ coll? (cond-> (doall (instrument-coll form))
+ (::do-break (meta form)) (with-break))
;; Other things are uninteresting, literals or unreadable objects.
- form))
+ (cond-> form
+ (::do-break (meta form)) (with-break))))
;;;; ## Pre-instrumentation
;;;
@@ -294,9 +296,14 @@
"Tag form to be instrumented with breakfunction.
This sets the ::breakfunction metadata of form, which can then be
used by `instrument-tagged-code`. See this function for the meaning
- of breakfunction."
- [form breakfunction]
- (m/merge-meta form {::breakfunction breakfunction}))
+ of breakfunction.
+ When `do-break?` is true it tells the instrumenter to wrap the form
+ with a breakpoint regardless of other heuristics."
+ ([form breakfunction]
+ (tag-form form breakfunction false))
+ ([form breakfunction do-break?]
+ (m/merge-meta form {::breakfunction breakfunction}
+ (when do-break? {::do-break true}))))
(defn tag-form-recursively
"Like `tag-form` but also tag all forms inside the given form."
diff --git a/src/data_readers.clj b/src/data_readers.clj
index c865dc09..713b31f8 100644
--- a/src/data_readers.clj
+++ b/src/data_readers.clj
@@ -1,5 +1,5 @@
{dbg cider.nrepl.middleware.debug/debug-reader
break cider.nrepl.middleware.debug/breakpoint-reader
- exn cider.nrepl.middleware.debug/break-on-exception-reader
- dbgexn cider.nrepl.middleware.debug/debug-on-exception-reader
+ break! cider.nrepl.middleware.debug/break-on-exception-reader
+ dbg! cider.nrepl.middleware.debug/debug-on-exception-reader
light cider.nrepl.middleware.enlighten/light-reader}
diff --git a/test/clj/cider/nrepl/middleware/content_type_test.clj b/test/clj/cider/nrepl/middleware/content_type_test.clj
index 7c2d02aa..96677f3c 100644
--- a/test/clj/cider/nrepl/middleware/content_type_test.clj
+++ b/test/clj/cider/nrepl/middleware/content_type_test.clj
@@ -44,14 +44,15 @@
[:body :content-type :content-transfer-encoding :status]))))
(testing "java.io.File"
- (let [f (java.io.File/createTempFile "foo" ".txt")]
+ (let [f (java.io.File/createTempFile "foo" ".txt")
+ path (.getCanonicalPath f)]
(is (= {:body ""
:content-type
["message/external-body"
- {:URL (str "file:" f) :access-type "URL"}]
+ {:URL (str "file:" path) :access-type "URL"}]
:status #{"done"}}
(-> {:op "eval"
- :code (str "(java.io.File. " (pr-str (str f)) ")")
+ :code (str "(java.io.File. " (pr-str path) ")")
:content-type "true"}
session/message
(select-keys [:body :content-type :content-transfer-encoding :status]))))))
diff --git a/test/clj/cider/nrepl/middleware/debug_integration_test.clj b/test/clj/cider/nrepl/middleware/debug_integration_test.clj
index e5b574a8..f4ee21fa 100644
--- a/test/clj/cider/nrepl/middleware/debug_integration_test.clj
+++ b/test/clj/cider/nrepl/middleware/debug_integration_test.clj
@@ -136,18 +136,59 @@
(f)))))
(deftest debug-expression-test
+ (--> :eval "(ns user.test.debug)")
+ (<-- {:ns "user.test.debug"})
+ (<-- {:status ["done"]})
+
(testing "normal eval (no debugging)"
(--> :eval "(+ 2 3)")
(<-- {:value "5"})
(<-- {:status ["done"]}))
- (testing "#break reader, no breakpoints"
- ;; This code has only literals and core functions, so it should
- ;; not break, but should just return the value
+ (testing "Top level breakpoints do not trigger"
+ ;; Since triggering a breakpoint to inspect the toplevel value is no different
+ ;; from returning the value.
(--> :eval "#break (+ 2 3)")
(<-- {:value "5"})
(<-- {:status ["done"]}))
+ (testing "#break on return value of call"
+ (--> :eval "(do #break (+ 2 3) :ok)")
+ (<-- {:debug-value "5"})
+ (--> :next)
+ (<-- {:value ":ok"})
+ (<-- {:status ["done"]}))
+
+ (testing "#dbg reader on uninteresting forms"
+ ;; The return value of a def form is uninteresting, as well as the sub forms
+ ;; which are just core vars and literals.
+ ;; #dbg should not break on any of them and just return the value.
+ (--> :eval "(do #dbg (def foo [2 + {:skip \"me\"}]) foo)")
+ (<-- {:value "[2 #function[clojure.core/+] {:skip \"me\"}]"})
+ (<-- {:status ["done"]}))
+
+ (testing "#break reader on uninteresting forms"
+ ;; #break signifies that the user explicitly wants to set a breakpoint there,
+ ;; so ignore any of #dbg reader's heuristics on what an interesting form is.
+ (--> :eval "(let [a 1] #break [2 3], #break {:a a}, #break + #break (def foo 4) foo) ")
+ (<-- {:debug-value "[2 3]"})
+ (--> :next)
+ (<-- {:debug-value "{:a 1}"})
+ (--> :next)
+ (<-- {:debug-value "#function[clojure.core/+]"})
+ (--> :next)
+ (<-- {:debug-value "#'user.test.debug/foo"})
+ (--> :next)
+ (<-- {:value "4"})
+ (<-- {:status ["done"]}))
+
+ (testing "#break reader on literals"
+ ;; The instrumenter cannot tag literals which don't support metadata,
+ ;; so #break does not work on them.
+ (--> :eval "(do #break :kwd, #break 123, #break \"string\" :ok)")
+ (<-- {:value ":ok"})
+ (<-- {:status ["done"]}))
+
(testing "#dbg reader, with breaks"
(--> :eval
"#dbg
diff --git a/test/clj/cider/nrepl/middleware/info_test.clj b/test/clj/cider/nrepl/middleware/info_test.clj
index d02c2504..62ca98a8 100644
--- a/test/clj/cider/nrepl/middleware/info_test.clj
+++ b/test/clj/cider/nrepl/middleware/info_test.clj
@@ -310,8 +310,8 @@
(testing "java interop method with multiple classes"
(let [response (session/message {:op "eldoc" :sym ".length" :ns "cider.nrepl.middleware.info-test"})]
- (is (= (:class response)
- ["java.lang.String" "java.lang.StringBuffer" "java.lang.CharSequence" "java.lang.StringBuilder"])
+ (is (every? (set (:class response)) ;; is a superset of:
+ ["java.lang.String" "java.lang.StringBuffer" "java.lang.CharSequence" "java.lang.StringBuilder"])
(pr-str response))
(is (= (:member response) "length"))
(is (not (contains? response :ns)))