Skip to content

Debug on exception #3337

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

Closed
wants to merge 3 commits into from
Closed

Conversation

bo-tato
Copy link

@bo-tato bo-tato commented Apr 26, 2023

Add support on the cider side to go with clojure-emacs/cider-nrepl#769
passing a negative argument to cider-eval-defun-at-point will instrument it with #dbgexn
also added some documentation to the debugger docs describing the new feature


Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • All code passes the linter (eldev lint) which is based on elisp-lint and includes
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the user manual (if adding/changing user-visible functionality)

Thanks!

If you're just starting out to hack on CIDER you might find this section of its
manual
extremely useful.

@bbatsov
Copy link
Member

bbatsov commented Jun 21, 2023

@yuhan0 I guess you'll take this from here, right?

@yuhan0
Copy link
Contributor

yuhan0 commented Jun 21, 2023

How do you feel about this proposal of overloading cider-eval-defun with a negative argument?
I find that sort of API quite bad for discoverability - in my personal config I have these custom commands instead, along with similar ones for various other temporary debugging forms - basically generalizing the existing cider--prompt-and-insert-inline-dbg interface.

;; * Insert #break / #dbg reader forms
(defun +cider-insert-dbg (ex)
  "Insert #dbg reader.
If EX is non nil or called with prefix arg, insert #dbg! to break on exception.
Automatically reevaluates form if inside a defn."
  (interactive "P")
  (unless (looking-at-p "(")
    (backward-up-list))
  (cider-insert-fragile-reader-macro
   (if ex "#dbg!"
     (concat "#dbg"
             (let ((cond (cider-read-from-minibuffer "Condition: ")))
               (unless (equal "" cond)
                 (format " ^{:break/when %s}" cond))))))
  (when (clojure-top-level-form-p "defn")
    (cider-eval-defun-at-point)))

(defun +cider-insert-breakpoint (ex)
  "Insert #break reader.
If EX is non nil or called with prefix arg, insert #break! to break on exception."
  (interactive "P")
  (cider-insert-fragile-reader-macro
   (if ex "#break!"
     (concat "#break"
             (let ((cond (cider-read-from-minibuffer "Condition: ")))
               (unless (equal "" cond)
                 (format " ^{:break/when %s}" cond)))))))

@yuhan0
Copy link
Contributor

yuhan0 commented Jun 21, 2023

@bbatsov
Copy link
Member

bbatsov commented Jun 21, 2023

How do you feel about this proposal of overloading cider-eval-defun with a negative argument?

I'm not a big fan of prefix arguments in general, but from time to time they fit in nicely. Definitely not a big fan of the negative argument, as it's probably the hardest to discover organically by users. I'm ok with adding some special commands and I guess we can have something with prefix arguments as well, for whoever prefers them, given that it adds little complexity.

The demo looks good to me.

@bbatsov
Copy link
Member

bbatsov commented Jul 5, 2023

@yuhan0 Any chance you'll have time for this soon? I think it's the only feature remaining for the next CIDER release.

@yuhan0
Copy link
Contributor

yuhan0 commented Jul 5, 2023

Yes, I have the working code for this feature but unfortunately don't have much time over the next couple of days to work on testing / refining it into a PR. If it's a blocker for a release maybe I'll push those commits now and see if anyone can take it from there? Might be able to do something over the weekend too.

@yuhan0 yuhan0 mentioned this pull request Jul 5, 2023
6 tasks
@bbatsov
Copy link
Member

bbatsov commented Jul 6, 2023

@yuhan0 No rush, there's one more blocker anyways (#3365). If you can spend some time on this over the weekend that'd be great.

I'll close this PR in favor of #3366 that you've opened just now.

@bbatsov bbatsov closed this Jul 6, 2023
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.

3 participants