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

Fix cider-eval-defun-up-to-point incorrectly matching brackets #3031

Merged

Conversation

LuisThiamNye
Copy link
Contributor

@LuisThiamNye LuisThiamNye commented Aug 9, 2021

Consider:

(prn 1 [2 {3 4|} 5] 6)

If | represents the cursor, and then I run cider-eval-defun-up-to-point, CIDER attempts to evaluate (prn 1 [2 {3 4|)]} resulting in an exception.

I also get the issue in a comment block:

(comment
  (prn 5|)
)

...which results in the evaluation of (prn 5)) — there is an extra closing delimiter as cider--calculate-closing-delimiters looks beyond the prn form and matches the ( of the comment form. I would expect the evaluation of (prn 5).

In this patch I have integrated modified functions that appear to have solved this problem for me.

The code for the new cider--insert-closing-delimiters is an adaptation of the cider-repl-closing-return function. Perhaps cider-repl-closing-return should depend on the more general cider--insert-closing-delimiters?

In one commit I have removed functions that seem to have been made redundant.


  • 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)

- Completes the different types of brackets in the
correct order (previously they were reversed).
- Works when evaluating top-level forms inside
Rich Comment Blocks.
No longer needed due to change in
cider-eval-defun-up-to-point
@LuisThiamNye LuisThiamNye marked this pull request as ready for review August 9, 2021 23:42
cider-eval.el Outdated Show resolved Hide resolved
@bbatsov
Copy link
Member

bbatsov commented Aug 25, 2021

Perhaps cider-repl-closing-return should depend on the more general cider--insert-closing-delimiters?

Yeah, you're probably right about it. It simply predates the cider--insert-closing-delimiters function.

Overall your changes look good; I left only a couple of small remarks.

cider-eval.el Outdated Show resolved Hide resolved
@LuisThiamNye LuisThiamNye force-pushed the fix-cider-eval-defun-up-to-point branch from d260b4d to 3bb24cd Compare August 25, 2021 18:43
cider--insert-closing-delimiters now takes and returns code
@LuisThiamNye LuisThiamNye force-pushed the fix-cider-eval-defun-up-to-point branch from 3bb24cd to 5edb90f Compare August 25, 2021 18:54
@bbatsov bbatsov merged commit fd5232d into clojure-emacs:master Aug 25, 2021
bbatsov pushed a commit that referenced this pull request Aug 25, 2021
@bbatsov
Copy link
Member

bbatsov commented Aug 25, 2021

Thanks for the quick follow-up! 🙇‍♂️

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.

2 participants