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

nrepl-bdecode-buffer did not handle negative integers. #577

Merged
merged 1 commit into from
May 23, 2014

Conversation

violahs
Copy link
Contributor

@violahs violahs commented May 23, 2014

This threw a silent (and therefore nasty) bug that froze the repl when a large
stack trace contained a negative number.

Regex now modified to accept an optional leading negative.

This threw a silent (and therefore nasty) bug that froze the repl when a large
stack trace contained a negative number.

Regex now modified to accept an optional leading negative.
@@ -185,7 +185,7 @@ To be used for tooling calls (i.e. completion, eldoc, etc)")
;;; and modified to work with utf-8
(defun nrepl-bdecode-buffer ()
"Decode a bencoded string in the current buffer starting at point."
(cond ((looking-at "i\\([0-9]+\\)e")
(cond ((looking-at "i\\(-*[0-9]+\\)e")
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be -? as there will always be just one - or none.

@bbatsov
Copy link
Member

bbatsov commented May 23, 2014

You should also add a test for that.

bbatsov added a commit that referenced this pull request May 23, 2014
nrepl-bdecode-buffer did not handle negative integers.
@bbatsov bbatsov merged commit c261763 into clojure-emacs:master May 23, 2014
bbatsov pushed a commit that referenced this pull request May 23, 2014
@bbatsov
Copy link
Member

bbatsov commented May 23, 2014

I added a test myself just now, since it turned out this was related to another major problem (clojure-emacs/cider-nrepl#68) and I wanted to merge your fix ASAP. Thanks a lot for hunting this down!

@violahs
Copy link
Contributor Author

violahs commented May 23, 2014

I found this bug precisely because of that problem (stack trace). So its really the same user issue.

@jeffvalk
Copy link
Contributor

@violahs Good find. This was a particularly nasty one. Quite a headache for just 2 characters...

dgtized pushed a commit to dgtized/cider that referenced this pull request Jun 24, 2014
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.

4 participants