Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debugging in Cider #1019

Merged
merged 2 commits into from
Mar 28, 2015
Merged

Debugging in Cider #1019

merged 2 commits into from
Mar 28, 2015

Conversation

Malabarba
Copy link
Member

This PR goes along with clojure-emacs/cider-nrepl#170, please see there for instructions.

cider-debug

@bbatsov
Copy link
Member

bbatsov commented Mar 15, 2015

Adding a menu for this would be nice. You might also post an animated gif showcasing the prototype as some users might have a hard time updating the client and the middleware by themselves.

@Malabarba
Copy link
Member Author

Added gif on both posts.

@Malabarba
Copy link
Member Author

When you say menu, do you mean an item under the Cider menu-bar?

@bbatsov
Copy link
Member

bbatsov commented Mar 15, 2015

Yes. And maybe a separate debug menu when the debugger is enabled (as in
edebug).

On 15 March 2015 at 18:16, Artur Malabarba notifications@github.com wrote:

When you say menu, do you mean an item under the Cider menu-bar?


Reply to this email directly or view it on GitHub
#1019 (comment).

Best Regards,
Bozhidar Batsov

http://www.batsov.com

@@ -786,6 +786,26 @@ for functionality like pretty-printing won't clobber the values of *1, *2, etc."
(defvar nrepl-err-handler 'cider-default-err-handler
"Evaluation error handler.")

(defvar-local nrepl--input-handler-queue nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, nrepl-client.el already loads queue.el. You might consider using that data structure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer. Will do.

@vspinu
Copy link
Contributor

vspinu commented Mar 19, 2015

@Bruce-Connor thanks. Truly great job!

Let's bind this to C-u C-M-x to mimic edebug. Printing could be shifted to C-u C-u. Not a big deal.

(nrepl-request:stdin
;; For now we immediately try to read-char. Ideally, this will
;; be done in a minor-mode (like edebug does) so that the user
;; isn't blocked from doing anything else.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and also bound to some rarely used but easelly accessible combos (like C-Q and C-C) to avoidinterference with the editing. The only thing I dislike about edebug is the read-only lock.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making it read-only is a precaution we might need as well. The way it's currently designed, if the user edits the source file inside or before the instrumented sexp, then the debugger will start to misbehave. It will probably place point in the wrong places and might end up throwing an error while trying to navigate the code (due to calling forward-sexp in an invalid location).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the user edits the source file inside or before the instrumented sexp, then the debugger will start to misbehave.

The debugging can be aborted if that happens, similarly to what edebug does when the source has been modified.

Alternatively we can check the region for modifications, and if there have been such since the last instrumentation, re-instrument the definition in the background. Not sure if that's possible at debugger.core level.

@Malabarba
Copy link
Member Author

Let's bind this to C-u C-M-x to mimic edebug. Printing could be shifted to C-u C-u. Not a big deal.

I agree. Printing can also always be done with C-u C-x C-e anyway.

@bbatsov
Copy link
Member

bbatsov commented Mar 26, 2015

@Malabarba Love the new handle. :-)

@Malabarba
Copy link
Member Author

Thanks. Just hoping I'll stop being called Bruce now.
Even changed the email on my profile to the blog's domain, even though it only does forwarding. =P

@bbatsov
Copy link
Member

bbatsov commented Mar 26, 2015

Well, to be honest I thought your name was Bruce Connor for a while myself. :-)

@Malabarba
Copy link
Member Author

Exactly. :-) It's my own fault, of course.
Just an old nick from my gaming era which stuck around a little too long.

@Malabarba
Copy link
Member Author

Squashed. I can remove the keybind if it's not ok.

@bbatsov
Copy link
Member

bbatsov commented Mar 28, 2015

Please, drop the . in the second commit's title (titles are not sentences) and mention the debugger in both the changelog and the readme.

@Malabarba
Copy link
Member Author

Please, drop the . in the second commit's title (titles are not sentences) and mention the debugger in
both the changelog and the readme.

Done

@bbatsov
Copy link
Member

bbatsov commented Mar 28, 2015

Hmm, now the branch can't be merged. Guess it was not in sync with the current master, so you'll have to rebase.

It allows cider modules to specify a function that will handle the next
need-input event.
Add cider-debug-defun-at-point to the menu-bar.
Bind cider-debug-defun-at-point to cider-eval-defun-at-point with a prefix.

This needs debug.clj at cider-nrepl to work.
Invoke M-x cider-debug-defun-at-point to see it in action.
@Malabarba
Copy link
Member Author

Rebased

bbatsov added a commit that referenced this pull request Mar 28, 2015
@bbatsov bbatsov merged commit b22a523 into clojure-emacs:master Mar 28, 2015
@bbatsov
Copy link
Member

bbatsov commented Mar 28, 2015

🎉

@Malabarba
Copy link
Member Author

🍺

@arichiardi
Copy link
Contributor

Good job!

@arrdem
Copy link
Contributor

arrdem commented Mar 28, 2015

🍻

@DesmondHayes
Copy link

Thanks for the great work. Happy to see debugging capabilities in Cider.
Here, I've found some issues for you to solve:) (Should I open a new issue?):
1.

(defn factorial
  [n]
  (loop [cnt n acc 1]
    (if (zero? cnt)
      acc
      (recur (dec cnt) (* acc cnt)))))

C-u C-c C-c to debug factorial gives an error in cider buffer:

Failed to instrument  (recur # #)
CompilerException java.lang.UnsupportedOperationException: Can only recur from tail position, compiling:(blah-blah-blah)
(defn foo
  [v]
  (do v))

C-u C-c C-c to debug foo gives an error in cider buffer:

Failed to instrument  (do v)

and also sometimes hangs cider repl so that only C-c C-c can stop it (at which point it gives a huge error with a stacktrace).

@bbatsov
Copy link
Member

bbatsov commented Mar 31, 2015

Open a new issue.

On Tuesday, March 31, 2015, Desmond Hayes notifications@github.com wrote:

Thanks for the great work. Happy to see debugging capabilities in Cider.
Here, I've found some issues for you to solve:) (Should I open a new
issue?):
1.

(defn factorial
[n](loop [cnt n acc 1]
%28if %28zero? cnt%29
acc
%28recur %28dec cnt%29 %28* acc cnt%29%29%29))

C-u C-c C-c to debug factorial gives an error in cider buffer:

Failed to instrument (recur # #)
CompilerException java.lang.UnsupportedOperationException: Can only recur from tail position, compiling:(blah-blah-blah)

(defn foo
[v](do v))

C-u C-c C-c to debug foo gives an error in cider buffer:

Failed to instrument (do v)

and also sometimes hangs cider repl so that only C-c C-c can stop it (at
which point it gives a huge error with a stacktrace).


Reply to this email directly or view it on GitHub
#1019 (comment).

Best Regards,
Bozhidar Batsov

http://www.batsov.com

@Malabarba
Copy link
Member Author

@DesmondHayes thanks for catching these, could you open a new issue for them?

Item 2 is probably an easy fix, we should be able to instrument that for sure.
Item 1 is more of a conceptual challenge. We'll have to discuss a solution.

@Malabarba
Copy link
Member Author

@DesmondHayes Nevermind. These should both be fixed on PR clojure-emacs/cider-nrepl#177

@DesmondHayes
Copy link

@Malabarba I've actually opened issue #1048. Also, have you seen #1049?

@Malabarba Malabarba deleted the debug branch April 1, 2015 14:52
@johnbendi
Copy link

Please how do I turn it off?

@Malabarba
Copy link
Member Author

@johnbendi What do you mean by that? The debugger is off until you activate it.

@johnbendi
Copy link

                                                                                  @malabarba how do I deactivate?                                                                                                                                                                                                                                                                                                                                         Sent from my BlackBerry 10 smartphone.                                                                                                                                                                                                                From: Artur MalabarbaSent: Saturday, June 13, 2015 7:48 AMTo: clojure-emacs/ciderReply To: clojure-emacs/ciderCc: John ChijiokeSubject: Re: [cider] Debugging in Cider (#1019)@johnbendi What do you mean by that? The debugger is off until you activate it.

—Reply to this email directly or view it on GitHub.

@Malabarba
Copy link
Member Author

@johnbendi

  • If you mean "how do I quit in the middle of debugging something", then hit the q key.
  • If you mean "after I debug a function how do I stop it from activating the debugger every time it is called again?", then you just need to evaluate the function's defn normally (with C-M-x).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants