From a06ceade05cbaa0295a980638a352a29fbde80a3 Mon Sep 17 00:00:00 2001 From: Chris Perkins Date: Thu, 10 Mar 2016 05:27:39 -0700 Subject: [PATCH] Add optional force parameter to :out and :here ops When an `:out` or `:here` operation jumps past calls to other instrumented functions, the debugger will normally stop in those functions. Passing a truthy `force` parameter to one of these ops will cause the debugger to skip all breaks outside of the currently debugged form, effecively forcing it to immediately step `:out` or `:here`. --- src/cider/nrepl/middleware/debug.clj | 71 ++++++---- .../middleware/debug_integration_test.clj | 131 ++++++++++++++---- .../clj/cider/nrepl/middleware/debug_test.clj | 2 +- 3 files changed, 147 insertions(+), 57 deletions(-) diff --git a/src/cider/nrepl/middleware/debug.clj b/src/cider/nrepl/middleware/debug.clj index 2a92af52e..c0d888304 100644 --- a/src/cider/nrepl/middleware/debug.clj +++ b/src/cider/nrepl/middleware/debug.clj @@ -98,30 +98,38 @@ (defn skip-breaks? "True if the breakpoint at coordinates should be skipped. - If the mode of `*skip-breaks*` is :all, return true. - - Otherwise, the mode should be :deeper, :before, or :trace. - If :deeper, return true if the given coordinates are deeper than the - coordinates stored in `*skip-breaks*`. - If :before, return true if they represent a place before the coordinates - in `*skip-breaks*`. - If :trace, return false." + + The `*skip-breaks*` map stores a `mode`, `coordinates`, the `code` that it + applies to, and a `force?` flag. Behaviour depends on the `mode`: + - :all - return true, skipping all breaks + - :trace - return false, skip nothing + - :deeper - return true if the given coordinates are deeper than the + coordinates stored in `*skip-breaks*`, in the same code + - :before - return true if the given coordinates represent a place before + the coordinates in `*skip-breaks*`, in the same code + + For :deeper and :before, if we are not in the same code (i.e. we have stepped + into another instrumented function), then return the value of `force?`." [coordinates] (if (seq coordinates) - (when-let [{mode :mode skip-coords :coor code :code} @*skip-breaks*] - (case mode - ;; From :continue, skip everything. - :all true - ;; From :trace, never skip. - :trace false - ;; From :out, skip some breaks. - :deeper (let [parent (take (count skip-coords) coordinates)] - (and (= code (:code *extras*)) - (seq= skip-coords parent) - (> (count coordinates) (count parent)))) - ;; From :here, skip some breaks. - :before (and (= code (:code *extras*)) - (coord< coordinates skip-coords)))) + (when-let [{mode :mode skip-coords :coor + code-to-skip :code force? :force?} @*skip-breaks*] + (let [same-defn? (identical? code-to-skip (:code *extras*))] + (case mode + ;; From :continue, skip everything. + :all true + ;; From :trace, never skip. + :trace false + ;; From :out, skip some breaks. + :deeper (if same-defn? + (let [parent (take (count skip-coords) coordinates)] + (and (seq= skip-coords parent) + (> (count coordinates) (count parent)))) + force?) + ;; From :here, skip some breaks. + :before (if same-defn? + (coord< coordinates skip-coords) + force?)))) ;; We don't breakpoint top-level sexps, because their return value ;; is already displayed anyway. true)) @@ -130,16 +138,16 @@ "Set the value of *skip-breaks* for the top-level breakpoint. Additional arguments depend on mode, and should be: - empty for :all or :trace - - coordinates and code for :deeper or :before + - coordinates, code, and force for :deeper or :before See `skip-breaks?`." ([mode] - (skip-breaks! mode nil nil)) - ([mode coor code] + (skip-breaks! mode nil nil nil)) + ([mode coor code force?] (reset! *skip-breaks* (case mode (nil false) nil (:all :trace) {:mode mode} - (:deeper :before) {:mode mode :coor coor :code code})))) + (:deeper :before) {:mode mode :coor coor :code code :force? force?})))) (defn- abort! "Stop current eval thread. @@ -305,16 +313,16 @@ (cljs/grab-cljs-env *msg*) identity) response-raw (read-debug extras commands nil) - {:keys [code coord response page-size] :or {page-size 32}} + {:keys [code coord response page-size force?] :or {page-size 32}} (if (map? response-raw) response-raw {:response response-raw}) extras (dissoc extras :inspect)] (case response :next value :continue (do (skip-breaks! :all) value) - :out (do (skip-breaks! :deeper (butlast (:coor extras)) (:code extras)) + :out (do (skip-breaks! :deeper (butlast (:coor extras)) (:code extras) force?) value) - :here (do (skip-breaks! :before coord (:code extras)) + :here (do (skip-breaks! :before coord (:code extras) force?) value) :locals (inspect-then-read-command value extras page-size *locals*) :inspect (->> (read-debug-eval-expression "Inspect value: " extras code) @@ -388,7 +396,10 @@ ;; once. Next time, we need to test the condition again. (binding [*skip-breaks* (if ~condition *skip-breaks* - (atom {:mode :deeper :coor ~parent-coor :code ~(:code *msg*)}))] + (atom {:mode :deeper + :coor ~parent-coor + :code ~(:code *msg*) + :force? false}))] (breakpoint-implementation ~form ~extras ~original-form))) `(with-debug-bindings ~extras (breakpoint-implementation ~form ~extras ~original-form))))) diff --git a/test/clj/cider/nrepl/middleware/debug_integration_test.clj b/test/clj/cider/nrepl/middleware/debug_integration_test.clj index 2fc31c154..6f834f64c 100644 --- a/test/clj/cider/nrepl/middleware/debug_integration_test.clj +++ b/test/clj/cider/nrepl/middleware/debug_integration_test.clj @@ -94,11 +94,13 @@ (nrepl-send {:op :debug-input :input ~(str op) :key (current-key)}))) (def-debug-op :next) -(def-debug-op :out) (def-debug-op :continue) -(defmethod debugger-send :here [_ coor] - (nrepl-send {:op :debug-input :input (str {:response :here :coord coor}) :key (current-key)})) +(defmethod debugger-send :out [_ & [force?]] + (nrepl-send {:op :debug-input :input (str {:response :out :force? force?}) :key (current-key)})) + +(defmethod debugger-send :here [_ coor & [force?]] + (nrepl-send {:op :debug-input :input (str {:response :here :coord coor :force? force?}) :key (current-key)})) (defmacro debugger-expect [expected] ;; This is a macro so that failed assertions will show at the right @@ -273,33 +275,110 @@ (<-- {:status ["done"]}) ;; Then eval some code that calls the function - (let [code "#dbg + (--> :eval "#dbg (let [i 7] (foo (inc i)) - (dec i))"] - (--> :eval code) - - ;; 1) should break at the `i` in `(foo (inc i))` - (<-- {:debug-value "7" :coor [2 1 1] :locals [["i" "7"]]}) - - ;; 2) Do a `:here` op at the end of `(dec i)`, skipping over the - ;; call to foo. - (--> :here [3]) - - ;; 3) The debugger should stop in `foo` - (<-- {:debug-value "8" :coor [3 2] :locals [["x" "8"]]}) - (--> :next) - (<-- {:debug-value "18" :coor [3] :locals [["x" "8"]]}) - (--> :next) + (dec i))") - ;; 4) Then, once out of foo, should jump to the point where we did - ;; the `:here` op. - (<-- {:debug-value "6" :coor [3]}) - (--> :next) + ;; 1) should break at the `i` in `(foo (inc i))` + (<-- {:debug-value "7" :coor [2 1 1] :locals [["i" "7"]]}) + + ;; 2) Do a `:here` op at the end of `(dec i)`, skipping over the + ;; call to foo. + (--> :here [3]) - ;; 5) return value of the `let` - (<-- {:value "6"}) - (<-- {:status ["done"]}))) + ;; 3) The debugger should stop in `foo` + (<-- {:debug-value "8" :coor [3 2] :locals [["x" "8"]]}) + (--> :next) + (<-- {:debug-value "18" :coor [3] :locals [["x" "8"]]}) + (--> :next) + + ;; 4) Then, once out of foo, should jump to the point where we did + ;; the `:here` op. + (<-- {:debug-value "6" :coor [3]}) + (--> :next) + + ;; 5) return value of the `let` + (<-- {:value "6"}) + (<-- {:status ["done"]})) + + +;;; Tests for force-step operations + +(deftest force-step-out-past-instrumented-fn + ;; When we force-step out of a form, instrumented functions that are + ;; called should not be debugged. + + (--> :eval "(ns user.test.debug)") + (<-- {:ns "user.test.debug"}) + (<-- {:status ["done"]}) + + ;; First, create an instrumented function + (--> :eval + "#dbg + (defn foo [x] + (+ 10 x))") + (<-- {:value "#'user.test.debug/foo"}) + (<-- {:status ["done"]}) + + ;; Then eval some code that calls the function + (--> :eval + "#dbg + (let [i 4] + (inc i) + (foo i))") + + ;; 1) should break at the `i` and then the `(inc i)` + (<-- {:debug-value "4" :coor [2 1] :locals [["i" "4"]]}) + (--> :next) + (<-- {:debug-value "5" :coor [2] :locals [["i" "4"]]}) + + ;; 2) Now force-step out, to skip stepping through the rest of the + ;; `let` form, including the call to `foo`. + (--> :out true) + + ;; 3) The debugger should not stop in `foo`, even though it is + ;; instrumented, but should go straight to the value of the let + ;; form. + (<-- {:value "14"}) + (<-- {:status ["done"]})) + +(deftest force-here-past-instrumented-fn + ;; When we do force-here operation that jumps past a call to an + ;; instrumented function, the function should not be debugged. + + (--> :eval "(ns user.test.debug)") + (<-- {:ns "user.test.debug"}) + (<-- {:status ["done"]}) + + ;; First, create an instrumented function + (--> :eval + "#dbg + (defn foo [x] + (+ 10 x))") + (<-- {:value "#'user.test.debug/foo"}) + (<-- {:status ["done"]}) + + ;; Then eval some code that calls the function + (--> :eval + "#dbg + (let [i 5] + (foo (inc i)) + (dec i))") + + ;; 1) should break at the `i` in `(foo (inc i))` + (<-- {:debug-value "5" :coor [2 1 1] :locals [["i" "5"]]}) + + ;; 2) Now do a force-here to `(dec i)` + (--> :here [3] true) + + ;; 3) The debugger should not stop in `foo`, even though it is + ;; instrumented, but should stop after `(dec i)`. + (<-- {:debug-value "4" :coor [3]}) + (--> :next) + + (<-- {:value "4"}) + (<-- {:status ["done"]})) ;;; Tests for conditional breakpoints diff --git a/test/clj/cider/nrepl/middleware/debug_test.clj b/test/clj/cider/nrepl/middleware/debug_test.clj index a277d2633..a2f942ff3 100644 --- a/test/clj/cider/nrepl/middleware/debug_test.clj +++ b/test/clj/cider/nrepl/middleware/debug_test.clj @@ -39,7 +39,7 @@ (is (not (#'d/skip-breaks? [2 2 1]))) (let [code "(foo (bar blah x))"] - (#'d/skip-breaks! :deeper [1 2] code) + (#'d/skip-breaks! :deeper [1 2] code nil) (binding [d/*extras* {:code code}] (is (#'d/skip-breaks? [])) (is (not (#'d/skip-breaks? [1 2])))