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.

Sign up for GitHub

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

Jump to bottom

Enhanced error highlighting, errors in red, warnings in yellow. #307

Merged
merged 2 commits into from
Jul 24, 2013
Merged

Enhanced error highlighting, errors in red, warnings in yellow. #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d19b5b5
Enhanced error highlighting, errors in red, warnings in yellow.
Mar 15, 2013
5d080d6
fix handling of missing column better
Mar 20, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@


### New features ### New features


* Add highlighting of compilation warnings in addition to existing highlighting of errors

### Bugs fixed ### Bugs fixed


## 0.1.7 / 2013-03-13 ## 0.1.7 / 2013-03-13
Expand Down
112 changes: 73 additions & 39 deletions nrepl.el
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ buffer will be hidden.")
"Face used to highlight compilation errors in Clojure buffers." "Face used to highlight compilation errors in Clojure buffers."
:group 'nrepl) :group 'nrepl)


(defface nrepl-warning-highlight-face
'((t (:inherit font-lock-warning-face :underline (:color "yellow"))))
"Face used to highlight compilation warnings in Clojure buffers."
:group 'nrepl)

(defmacro nrepl-propertize-region (props &rest body) (defmacro nrepl-propertize-region (props &rest body)
"Add PROPS to all text inserted by executing BODY. "Add PROPS to all text inserted by executing BODY.
More precisely, PROPS are added to the region between the point's More precisely, PROPS are added to the region between the point's
Expand Down Expand Up @@ -770,7 +775,7 @@ DONE-HANDLER, and EVAL-ERROR-HANDLER as appropriate."
(nrepl-emit-interactive-output value)) (nrepl-emit-interactive-output value))
(lambda (buffer err) (lambda (buffer err)
(message "%s" err) (message "%s" err)
(nrepl-highlight-compilation-error-line (nrepl-highlight-compilation-errors
buffer err)) buffer err))
'())) '()))


Expand All @@ -787,7 +792,7 @@ DONE-HANDLER, and EVAL-ERROR-HANDLER as appropriate."
(nrepl-emit-interactive-output value)) (nrepl-emit-interactive-output value))
(lambda (buffer err) (lambda (buffer err)
(message "%s" err) (message "%s" err)
(nrepl-highlight-compilation-error-line (nrepl-highlight-compilation-errors
buffer err)) buffer err))
'() '()
(lambda (buffer ex root-ex session) (lambda (buffer ex root-ex session)
Expand Down Expand Up @@ -849,10 +854,17 @@ Returns the position at which PROPERTY was found, or nil if not found."
ARG and RESET are ignored, as there is only ever one compilation error. ARG and RESET are ignored, as there is only ever one compilation error.
They exist for compatibility with `next-error'." They exist for compatibility with `next-error'."
(interactive) (interactive)
(let ((p (or (nrepl-find-property 'nrepl-note) (flet ((goto-next-note-boundary ()
(nrepl-find-property 'nrepl-note t)))) (let ((p (or (nrepl-find-property 'nrepl-note)
(when p (nrepl-find-property 'nrepl-note t))))
(goto-char p)))) (when p
(goto-char p)
(message (get-char-property p 'nrepl-note))))))
;; if we're already on a compilation error, first jump to the end of
;; it, so that we find the next error.
(when (get-char-property (point) 'nrepl-note)
(goto-next-note-boundary))
(goto-next-note-boundary)))


(defun nrepl-default-err-handler (buffer ex root-ex session) (defun nrepl-default-err-handler (buffer ex root-ex session)
"Make an error handler for BUFFER, EX, ROOT-EX and SESSION." "Make an error handler for BUFFER, EX, ROOT-EX and SESSION."
Expand All @@ -875,38 +887,59 @@ They exist for compatibility with `next-error'."
(with-current-buffer nrepl-error-buffer (with-current-buffer nrepl-error-buffer
(compilation-minor-mode +1)))))) (compilation-minor-mode +1))))))


(defun nrepl-highlight-compilation-error-line (buffer message) (defun nrepl-extract-error-info (regexp message)
Copy link
Member

Choose a reason for hiding this comment

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

The indentation of the body of the function seems pretty messed up.

"Extract error information with REGEXP against MESSAGE."
(let ((file (nth 1 regexp))
(line (nth 2 regexp))
(col (nth 3 regexp))
(type (nth 4 regexp))
(pat (first regexp)))
(when (string-match pat message)
;; special processing for type (1.2) style
(setq type (if (consp type)
(or (and (first type) (match-end (first type)) 1)
(and (rest type) (match-end (rest type)) 0)
2)))
(list
(when file
(let ((val (match-string-no-properties file message)))
(unless (string= val "NO_SOURCE_PATH") val)))
(when line (string-to-number (match-string-no-properties line message)))
(when col
(let ((val (match-string-no-properties col message)))
(when val (string-to-number val))))
(aref [nrepl-warning-highlight-face
nrepl-warning-highlight-face
nrepl-error-highlight-face]
(or type 2))
(when message)))))

(defun nrepl-highlight-compilation-errors (buffer message)
Copy link
Member

Choose a reason for hiding this comment

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

We cannot actually highlight more than 1 error, so I guess nrepl-highlight-compilation-error would be a more accurate name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually we can highlight more than one error, it's just that the compiler only generates at most one error at the moment. Maybe the compiler will at some future point emit more errors and we should be ready. There's a screenshot in the PR description with an example of a errror and warning on the same line.

Copy link
Member

Choose a reason for hiding this comment

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

Fair point.

"Highlight compilation error line in BUFFER, using MESSAGE." "Highlight compilation error line in BUFFER, using MESSAGE."
(with-current-buffer buffer (with-current-buffer buffer
(let ((error-line-number (nrepl-extract-error-line message)) (let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(error-filename (nrepl-extract-error-filename message))) (when info
(when (and (> error-line-number 0) (let ((file (nth 0 info))
(or (string= (buffer-file-name buffer) error-filename) (line (nth 1 info))
(string= "NO_SOURCE_PATH" error-filename))) (col (nth 2 info))
(save-excursion (face (nth 3 info))
;; when we don't have a filename the line number (note (nth 4 info)))
;; is relative to form start (save-excursion
(if (string= error-filename "NO_SOURCE_PATH") ;; when we don't have a filename the line number
(beginning-of-defun) ;; is relative to form start
;; else we go to the top of the file (if file
(goto-char (point-min))) (goto-char (point-min)) ; start of file
(forward-line (1- error-line-number)) (beginning-of-defun))
(let ((overlay (make-overlay (progn (back-to-indentation) (point)) (forward-line (1- line))
(progn (move-end-of-line nil) (point))))) ;; if have column, highlight sexp at that point otherwise whole line.
(overlay-put overlay 'nrepl-note-p t) (move-to-column (or col 0))
(overlay-put overlay 'face 'nrepl-error-highlight-face) (let ((begin (progn (if col (backward-up-list) (back-to-indentation)) (point)))
(overlay-put overlay 'nrepl-note message) (end (progn (if col (forward-sexp) (move-end-of-line nil)) (point))))
(overlay-put overlay 'help-echo message))))))) (let ((overlay (make-overlay begin end)))

(overlay-put overlay 'nrepl-note-p t)
(defun nrepl-extract-error-line (stacktrace) (overlay-put overlay 'face face)
"Extract the error line number from STACKTRACE." (overlay-put overlay 'nrepl-note note)
(string-match "compiling:(.+:\\([0-9]+\\)" stacktrace) (overlay-put overlay 'help-echo note)))))))))
(string-to-number (match-string 1 stacktrace)))

(defun nrepl-extract-error-filename (stacktrace)
"Extract the error filename from STACKTRACE."
(string-match "compiling:(\\(.+\\):" stacktrace)
(substring-no-properties (match-string 1 stacktrace)))


(defun nrepl-need-input (buffer) (defun nrepl-need-input (buffer)
"Handle an need-input request from BUFFER." "Handle an need-input request from BUFFER."
Expand Down Expand Up @@ -1561,12 +1594,13 @@ Useful in hooks."
Useful in hooks." Useful in hooks."
(nrepl-interaction-mode -1)) (nrepl-interaction-mode -1))


(defvar nrepl-compilation-regexps '("(\\([^)]+\\):\\([[:digit:]]+\\))" 1 2) (defvar nrepl-compilation-regexp
"Specifications for matching errors in Clojure stacktraces. '("\\(?:.*\\(warning, \\)\\|.*?\\(, compiling\\):(\\)\\([^:]*\\):\\([[:digit:]]+\\)\\(?::\\([[:digit:]]+\\)\\)?\\(\\(?: - \\(.*\\)\\)\\|)\\)" 3 4 5 (1))
"Specifications for matching errors and warnings in Clojure stacktraces.
See `compilation-error-regexp-alist' for help on their format.") See `compilation-error-regexp-alist' for help on their format.")


(add-to-list 'compilation-error-regexp-alist-alist (add-to-list 'compilation-error-regexp-alist-alist
(cons 'nrepl nrepl-compilation-regexps)) (cons 'nrepl nrepl-compilation-regexp))
(add-to-list 'compilation-error-regexp-alist 'nrepl) (add-to-list 'compilation-error-regexp-alist 'nrepl)


;;;###autoload ;;;###autoload
Expand Down
78 changes: 67 additions & 11 deletions test/nrepl-tests.el
View file
Open in desktop
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -135,17 +135,73 @@
(let ((nrepl-hide-special-buffers t)) (let ((nrepl-hide-special-buffers t))
(should (equal (nrepl-server-buffer-name) " *nrepl-server*")))) (should (equal (nrepl-server-buffer-name) " *nrepl-server*"))))


(ert-deftest test-nrepl-extract-error-line () (ert-deftest test-nrepl-extract-error-info-14 ()
(let ((st "this context, compiling:(/some/test/file.clj:20) ala bala")) (let ((message "CompilerException java.lang.RuntimeException: Unable to resolve symbol: dummy in this context, compiling:(/some/test/file/core.clj:31)"))
(should (= (nrepl-extract-error-line st) 20))) (let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(let ((st "this context, compiling:(NO_SOURCE_PATH:20) ala bala")) (should (string= (nth 0 info) "/some/test/file/core.clj"))
(should (= (nrepl-extract-error-line st) 20)))) (should (= (nth 1 info) 31))

(should (equal (nth 2 info) nil))
(ert-deftest test-nrepl-extract-error-filename () (should (equal (nth 3 info) 'nrepl-error-highlight-face)))))
(let ((st "this context, compiling:(/some/test/file.clj:20) ala bala"))
(should (string= (nrepl-extract-error-filename st) "/some/test/file.clj"))) (ert-deftest test-nrepl-extract-error-info-14-no-file ()
(let ((st "this context, compiling:(NO_SOURCE_PATH:20) ala bala")) (let ((message "CompilerException java.lang.RuntimeException: Unable to resolve symbol: dummy in this context, compiling:(NO_SOURCE_PATH:31)"))
(should (string= (nrepl-extract-error-filename st) "NO_SOURCE_PATH")))) (let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(should (equal (nth 0 info) nil))
(should (= (nth 1 info) 31))
(should (equal (nth 2 info) nil))
(should (equal (nth 3 info) 'nrepl-error-highlight-face)))))

(ert-deftest test-nrepl-extract-warning-info-14 ()
(let ((message "Reflection warning, /some/othertest/file/core.clj:24 - reference to field getCanonicalPath can't be resolved.
"))
(let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(should (string= (nth 0 info) "/some/othertest/file/core.clj"))
(should (= (nth 1 info) 24))
(should (equal (nth 2 info) nil))
(should (equal (nth 3 info) 'nrepl-warning-highlight-face)))))

(ert-deftest test-nrepl-extract-warning-info-14-no-file ()
(let ((message "Reflection warning, NO_SOURCE_PATH:24 - reference to field getCanonicalPath can't be resolved.
"))
(let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(should (equal (nth 0 info) nil))
(should (= (nth 1 info) 24))
(should (equal (nth 2 info) nil))
(should (equal (nth 3 info) 'nrepl-warning-highlight-face)))))

(ert-deftest test-nrepl-extract-error-info-15 ()
(let ((message "CompilerException java.lang.RuntimeException: Unable to resolve symbol: dummy in this context, compiling:(/some/test/file/core.clj:31:3)"))
(let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(should (string= (nth 0 info) "/some/test/file/core.clj"))
(should (= (nth 1 info) 31))
(should (= (nth 2 info) 3))
(should (equal (nth 3 info) 'nrepl-error-highlight-face)))))

(ert-deftest test-nrepl-extract-error-info-15-no-file ()
(let ((message "CompilerException java.lang.RuntimeException: Unable to resolve symbol: dummy in this context, compiling:(NO_SOURCE_PATH:31:3)"))
(let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(should (equal (nth 0 info) nil))
(should (= (nth 1 info) 31))
(should (= (nth 2 info) 3))
(should (equal (nth 3 info) 'nrepl-error-highlight-face)))))

(ert-deftest test-nrepl-extract-warning-info-15 ()
(let ((message "Reflection warning, /some/othertest/file/core.clj:24:43 - reference to field getCanonicalPath can't be resolved.
"))
(let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(should (string= (nth 0 info) "/some/othertest/file/core.clj"))
(should (= (nth 1 info) 24))
(should (= (nth 2 info) 43))
(should (equal (nth 3 info) 'nrepl-warning-highlight-face)))))

(ert-deftest test-nrepl-extract-warning-info-15-no-file ()
(let ((message "Reflection warning, NO_SOURCE_PATH:24:43 - reference to field getCanonicalPath can't be resolved.
"))
(let ((info (nrepl-extract-error-info nrepl-compilation-regexp message)))
(should (equal (nth 0 info) nil))
(should (= (nth 1 info) 24))
(should (= (nth 2 info) 43))
(should (equal (nth 3 info) 'nrepl-warning-highlight-face)))))


Copy link
Member

Choose a reason for hiding this comment

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

You should have tests covering names like NO_SOURCE_PATH and you should handle the possibility for a missing column number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed. I'll add them.

(defmacro nrepl-test-with-two-buffers (buffer-names &rest body) (defmacro nrepl-test-with-two-buffers (buffer-names &rest body)
(lexical-let ((create (lambda (b) (list b `(generate-new-buffer " *temp*"))))) (lexical-let ((create (lambda (b) (list b `(generate-new-buffer " *temp*")))))
Expand Down