Debugger fixes #552

vspinu · 2018-10-15T21:37:52Z

Two changes:

debugger forms are now localized in the sense that STATE__ is inserted where the reader form is located and not around the entire defun. Besides fixing non-closure situations as in Bugfix: Debugging deftype Forms #544 it is' likely to be useful in core-async contexts (macros with &env) for which there are a few issues open. Still have to check those.
implement a notion of a debugger session and build on top of it to fix a range of "continue" issues

There are now two continue commands, one local which stops on next #dbg or #break readers, and one global which behaves like the old continue. First one is now on c and the global on is on C.

I want to fix a few more debugger related issues but it seems wise to push these changes first so that people could start testing immediately.

clojure-emacs/cider-nrepl#552

bbatsov · 2018-10-15T22:03:59Z

@vspinu seems there's some real CI failure this time around:

src/cider/nrepl/middleware/debug.clj:367:24: wrong-arity: Function on var #'cider.nrepl.middleware.debug/skip-breaks! called with 1 args, but it is only known to take one of the following args: [atom mode] [atom mode coor code force?]

vspinu · 2018-10-15T22:10:14Z

Yes. A lot of tests rely heavily on debugger internals. Will have a look tomorrow. I was deceived by ein with-profile +plugin.mranderson/config test which does close to nothing. The readme should be updated to describe the new ways of running tests.

Will label WIP for time being.

bbatsov · 2018-10-15T22:16:26Z

I never used this - only lein with-profile +1.9,+test-clj test (and friends). But yeah, we should improve the readme in this regard. It'd be nice if lein test ran something meaningful as well.

bbatsov · 2018-10-16T08:19:04Z

src/cider/nrepl/middleware/debug.clj

 [orchard.meta :as m]
- [clojure.java.io :as io])
+ [clojure.java.io :as io]
+ [cider.nrepl.middleware.util.nrepl :refer [notify-client]])


We really need to sort the deps in this namespace. :-)

bbatsov · 2018-10-16T08:21:13Z

src/cider/nrepl/middleware/debug.clj

- '[clojure.tools.nrepl.middleware.session :as session]
- '[clojure.tools.nrepl.misc :refer [response-for]]
- '[clojure.tools.nrepl.transport :as transport])
+  '[clojure.tools.nrepl.middleware.interruptible-eval :refer [*msg*]]


The original indentation seems correct to me. Do you have non-default settings for indentation in clojure-mode?

No. Standard settings. The new indentation seems correct to me (2 spaces).

But I do see problems indenting macros in cider-nrepl project. The {:style/indent 1} in try-if-let in debug.clj is not taking effect. Could you please confirm?

For ordinary lists the indentation is 1-space. I just check this.

try-if-let is broken for me as well. I also notice it's not font-locked as a macro, which is an indication of a deeper problem with dynamic font-locking/indentation.

I actually had clojure-indent-style set to non-default value and that was causing the indent. Shall we add this to .dir-locals?

bbatsov · 2018-10-16T08:23:05Z

src/cider/nrepl/middleware/debug.clj

- {:status :eval-error
- :causes [(let [causes# (stacktrace/analyze-causes e# (:pprint-fn *msg*))]
- (when (coll? causes#) (last causes#)))]})))
+  {:status :eval-error


That seems like a another accidental change of indentation.

bbatsov · 2018-10-16T08:23:19Z

src/cider/nrepl/middleware/debug.clj

- :stacktrace (-> (Exception. "Dummy")
- (stacktrace/analyze-causes (:pprint-fn *msg*))
- last :stacktrace)}]}))
+  {:status :stack


bbatsov · 2018-10-16T08:24:10Z

src/cider/nrepl/middleware/debug.clj


 (def debug-commands
 {"c" :continue
+ "C" :Continue


I think we should probably give the keyword some different name. (more descriptive)

Any concrete ideas? I thought of :Continue-all or :Continue-non-stop but both of them take valuable horizontal space during the debugger display.

I think we should probably decouple this a bit from the UI (but that's an unrelated note), so we can provide a more compact view for people who actually know the commands.

I like :continue-non-stop, but I have to check with other debuggers. I'd also suggest using a lowercase letter as those are simpler to press.

I am quite indifferent on this but I think C is not bad here. It should be rarely used and C is suggestive that it's a more complex version of c command.

I have tried :continue-non-stop and it's pretty ugly and hypens don't help with legibility.

So I am leaving Continue for now till we get better ideas on this one.

The Debugger's menu says "Continue non stop", so I guess it should be fine as a first try.

bbatsov · 2018-10-16T08:25:08Z

src/cider/nrepl/middleware/debug.clj

- (assoc dbg-state :inspect)
- (recur value))
- (recur value dbg-state))
+  (->> (debug-inspect page-size val)


I think your indentation is incorrect in all try-if-let forms.

bbatsov · 2018-10-16T08:27:29Z

Apart from the few accidental indentation changes and a couple of small remarks the changes look good to me. Might be nice to add a couple of tests for them.

And as usual - amazing work! The users are going to be super excited about those improvements!

vspinu · 2018-10-16T13:19:09Z

I am no longer able to run cider tests interactively. With current master when run interactively tests in debug_integration_tests.clj fail but other tests which I have tried (appropos, format etc.) stall the nrepl.

Would you mind checking on your side if you could run any of the debug-interaction-tests by hand?

bbatsov · 2018-10-16T13:24:10Z

They freeze for me as well. No idea why, though. I can't think of any related changes.

bbatsov · 2018-10-16T16:59:56Z

Yeah, totally.

On Tue, 16 Oct 2018 at 18:52, Vitalie Spinu ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/cider/nrepl/middleware/debug.clj <#552 (comment)> : > (:import [clojure.lang Compiler$LocalBinding])) ;; Compatibility with the legacy tools.nrepl and the new nREPL 0.4.x. ;; The assumption is that if someone is using old lein repl or boot repl ;; they'll end up using the tools.nrepl, otherwise the modern one. (if (find-ns 'clojure.tools.nrepl) (require - '[clojure.tools.nrepl.middleware.interruptible-eval :refer [*msg*]] - '[clojure.tools.nrepl.middleware.session :as session] - '[clojure.tools.nrepl.misc :refer [response-for]] - '[clojure.tools.nrepl.transport :as transport]) + '[clojure.tools.nrepl.middleware.interruptible-eval :refer [*msg*]] I actually had clojure-indent-style set to non-default value and that was causing the indent. Shall we add this to .dir-locals? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#552 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGVytP4tvTjitItWjbicBGMx_-mV8s_ks5ulg7ggaJpZM4XdL_B> .

-- Best Regards, Bozhidar Batsov http://www.batsov.com

bbatsov · 2018-10-16T17:27:39Z

👌

On Tue, 16 Oct 2018 at 19:25, Vitalie Spinu ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/cider/nrepl/middleware/debug.clj <#552 (comment)> : > (def debug-commands {"c" :continue + "C" :Continue The Debugger's menu says "Continue non stop", so I guess it should be fine as a first try. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#552 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGVyokEYikjraWuvtQpUsZuC4PceNtaks5ulhaggaJpZM4XdL_B> .

-- Best Regards, Bozhidar Batsov http://www.batsov.com

Fix clojure-emacs/cider#2453 Close #544

Fix #460, fix clojure-emacs/cider#1869, fix clojure-emacs/cider#2253 fix clojure-emacs/cider#1054

vspinu · 2018-10-16T18:57:14Z

Should be good to go. There is an unrelated pprint failure in one job.

clojure-emacs/cider-nrepl#552

vspinu added a commit to vspinu/cider that referenced this pull request Oct 15, 2018

Differentiate between :continue and :Continue commands

09551ec

clojure-emacs/cider-nrepl#552

vspinu mentioned this pull request Oct 15, 2018

Adjustments to "continue" debugger UI clojure-emacs/cider#2485

Merged

vspinu added the wip label Oct 15, 2018

bbatsov reviewed Oct 16, 2018

View reviewed changes

vspinu force-pushed the localized-debugger branch from eba4840 to 0780d91 Compare October 16, 2018 17:29

vspinu added 3 commits October 16, 2018 19:38

Implement localized debugger state

24875ec

Fix clojure-emacs/cider#2453 Close #544

Use notify-client instead of println for debugger feedback

20f308a

Define :debugger lein test selector

44a0644

vspinu force-pushed the localized-debugger branch from 0780d91 to efa335d Compare October 16, 2018 17:45

Implement debugger session and fix continuation

06d0cd6

Fix #460, fix clojure-emacs/cider#1869, fix clojure-emacs/cider#2253 fix clojure-emacs/cider#1054

vspinu force-pushed the localized-debugger branch from efa335d to 06d0cd6 Compare October 16, 2018 17:53

bbatsov merged commit c2ef541 into master Oct 16, 2018

bbatsov deleted the localized-debugger branch October 16, 2018 22:13

bbatsov pushed a commit to clojure-emacs/cider that referenced this pull request Oct 16, 2018

Differentiate between :continue and :Continue commands

c936cda

clojure-emacs/cider-nrepl#552

johnv02139 pushed a commit to johnv02139/cider that referenced this pull request Oct 20, 2018

Differentiate between :continue and :Continue commands

06ca025

clojure-emacs/cider-nrepl#552

vspinu mentioned this pull request Nov 7, 2018

Fix instrumentation of recursive functions #560

Merged

vspinu removed the wip label Nov 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Debugger fixes #552

Debugger fixes #552

Uh oh!

vspinu commented Oct 15, 2018

bbatsov commented Oct 15, 2018

vspinu commented Oct 15, 2018

bbatsov commented Oct 15, 2018

bbatsov Oct 16, 2018

bbatsov Oct 16, 2018

vspinu Oct 16, 2018 •

edited

Loading

bbatsov Oct 16, 2018

vspinu Oct 16, 2018

bbatsov Oct 16, 2018

bbatsov Oct 16, 2018

bbatsov Oct 16, 2018

vspinu Oct 16, 2018

bbatsov Oct 16, 2018

vspinu Oct 16, 2018

vspinu Oct 16, 2018

vspinu Oct 16, 2018

bbatsov Oct 16, 2018

bbatsov commented Oct 16, 2018

vspinu commented Oct 16, 2018

bbatsov commented Oct 16, 2018

bbatsov commented Oct 16, 2018 via email

bbatsov commented Oct 16, 2018 via email

vspinu commented Oct 16, 2018

Labels

2 participants

Debugger fixes #552

Debugger fixes #552

Uh oh!

Conversation

vspinu commented Oct 15, 2018

bbatsov commented Oct 15, 2018

vspinu commented Oct 15, 2018

bbatsov commented Oct 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vspinu Oct 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbatsov commented Oct 16, 2018

vspinu commented Oct 16, 2018

bbatsov commented Oct 16, 2018

bbatsov commented Oct 16, 2018 via email

bbatsov commented Oct 16, 2018 via email

vspinu commented Oct 16, 2018

Labels

2 participants

vspinu Oct 16, 2018 •

edited

Loading