Show a rejection where it is, on the line it is actually on
An error that only reaches the echo area is gone the moment you type, and the location was the useful half of it. So the client draws an overlay at the `:loc` the daemon sent, with the message beside the code, and clears it the next time that buffer's evaluation is accepted — a marker left behind after a fix is a lie about the running program. Two things had to be right first, and neither was. The column in a `:loc` is a *byte* offset: lib/reader.ml walks the source a byte at a time and OCaml strings are bytes. The old code did `forward-char` with it, which is the same mistake as counting a frame's length in characters, in a different place — one accented character earlier on the line puts the marker as many columns to the right. It goes through `byte-to-position` from the line's start now, and is clamped to the end of the line, which the old code also needed: a column past a short line walked into the next one and pointed at innocent code. And the daemon numbers lines from the start of what it was *sent*, so `C-c C-c` on a defn halfway down a buffer came back saying line 1. Every overlay would have sat on the file's first line. The fix is leading newlines: the reader skips them, and the reply's line numbers are then the buffer's own. No protocol change, and nothing the daemon has to know. Marking the error must not itself signal — the error the caller is owed is the daemon's, and losing it to a bad location would report the wrong thing.
This commit is contained in:
Joseph Ferano
parent
a1285a5ac6
commit
aab6c28450
@ -23,6 +23,7 @@
|
||||
;;; Code:
|
||||
|
||||
(require 'subr-x)
|
||||
(require 'seq)
|
||||
|
||||
(defgroup flan-dev nil
|
||||
"Talking to a running Flan program."
|
||||
@ -173,6 +174,97 @@ With no argument, look for `flan-dev-socket-name' up from this buffer."
|
||||
(if (plist-get r :alive) "running" "exited")
|
||||
(length (plist-get r :fns)) (length (plist-get r :globals)))))
|
||||
|
||||
;;; Where an error is
|
||||
|
||||
;; A reply's :loc is "file:line:col", and the column is a *byte* offset into
|
||||
;; the line: the reader walks the source a byte at a time (lib/reader.ml), and
|
||||
;; OCaml strings are bytes. Emacs counts characters, so the same rule the
|
||||
;; framing has applies here — one non-ASCII character earlier on the line puts
|
||||
;; the marker as many columns to the right as that character has bytes. Going
|
||||
;; through `byte-to-position' from the line's start is the whole fix, and
|
||||
;; `forward-char' would also have walked into the next line on a column past
|
||||
;; the end of a short one.
|
||||
|
||||
(defun flan-dev--parse-loc (loc)
|
||||
"Split LOC, a \"file:line:col\" string, into (FILE LINE COL), or nil."
|
||||
(when (and (stringp loc)
|
||||
(string-match "\\`\\(.*\\):\\([0-9]+\\):\\([0-9]+\\)\\'" loc))
|
||||
(list (match-string 1 loc)
|
||||
(string-to-number (match-string 2 loc))
|
||||
(string-to-number (match-string 3 loc)))))
|
||||
|
||||
(defun flan-dev--position (line col)
|
||||
"Position of LINE and byte-column COL in the current buffer."
|
||||
(save-excursion
|
||||
(goto-char (point-min))
|
||||
(forward-line (1- line))
|
||||
(let* ((bol (point))
|
||||
(eol (line-end-position))
|
||||
(want (+ (position-bytes bol) (max 0 (1- col))))
|
||||
(p (and (<= want (position-bytes eol)) (byte-to-position want))))
|
||||
;; Clamped rather than trusted: a column past the end of the line is a
|
||||
;; location for something the reader wanted and did not find, and
|
||||
;; overshooting into the next line would point at innocent code.
|
||||
(min (or p eol) eol))))
|
||||
|
||||
(defun flan-dev--buffer-visiting (file)
|
||||
"The live buffer visiting FILE, or nil.
|
||||
Compared with `file-equal-p', so a symlinked or relative path still matches."
|
||||
(seq-find (lambda (b)
|
||||
(let ((n (buffer-local-value 'buffer-file-name b)))
|
||||
(and n (file-exists-p file) (file-equal-p n file))))
|
||||
(buffer-list)))
|
||||
|
||||
;;; Error overlays
|
||||
|
||||
;; An error is shown where it is rather than only in the echo area, because the
|
||||
;; echo area is gone the moment you type and the location is the useful half of
|
||||
;; the message. It is cleared when the next evaluation of that buffer is
|
||||
;; accepted: an overlay left behind after a fix is a lie about the program, and
|
||||
;; a stale one is worse than none.
|
||||
|
||||
(defface flan-dev-error-face
|
||||
'((t :inherit error :underline (:style wave)))
|
||||
"Face for the text an evaluation was rejected at."
|
||||
:group 'flan-dev)
|
||||
|
||||
(defface flan-dev-error-message-face
|
||||
'((t :inherit error :height 0.9))
|
||||
"Face for the message shown beside a rejected form."
|
||||
:group 'flan-dev)
|
||||
|
||||
(defun flan-dev-clear-errors (&optional buffer)
|
||||
"Remove Flan error overlays from BUFFER, or from the current buffer."
|
||||
(interactive)
|
||||
(with-current-buffer (or buffer (current-buffer))
|
||||
(remove-overlays (point-min) (point-max) 'flan-dev-error t)))
|
||||
|
||||
(defun flan-dev--show-error (loc msg)
|
||||
"Mark MSG at LOC, if LOC names a file some buffer is visiting.
|
||||
Returns non-nil when it put an overlay somewhere."
|
||||
(let ((parts (flan-dev--parse-loc loc)))
|
||||
(when parts
|
||||
(let ((buf (flan-dev--buffer-visiting (nth 0 parts))))
|
||||
(when buf
|
||||
(with-current-buffer buf
|
||||
(flan-dev-clear-errors buf)
|
||||
(let* ((beg (flan-dev--position (nth 1 parts) (nth 2 parts)))
|
||||
(end (save-excursion (goto-char beg) (line-end-position)))
|
||||
(ov (make-overlay beg end buf t nil)))
|
||||
(overlay-put ov 'flan-dev-error t)
|
||||
(overlay-put ov 'face 'flan-dev-error-face)
|
||||
(overlay-put ov 'help-echo msg)
|
||||
(overlay-put ov 'evaporate nil)
|
||||
(overlay-put ov 'priority 100)
|
||||
(overlay-put ov 'after-string
|
||||
(propertize (concat " " msg)
|
||||
'face 'flan-dev-error-message-face))
|
||||
;; Point goes there too, but only in the buffer being looked at:
|
||||
;; moving point in a buffer nobody is showing is a surprise the
|
||||
;; next time it is visited.
|
||||
(when (eq buf (current-buffer)) (goto-char beg))
|
||||
t)))))))
|
||||
|
||||
;;; Evaluating
|
||||
|
||||
(defun flan-dev--report (reply what)
|
||||
@ -182,6 +274,8 @@ With no argument, look for `flan-dev-socket-name' up from this buffer."
|
||||
(names (plist-get reply :names))
|
||||
(note (plist-get reply :note))
|
||||
(value (plist-get reply :value)))
|
||||
;; Accepted, so whatever the last rejection marked is no longer true.
|
||||
(flan-dev-clear-errors)
|
||||
(when flan-dev-echo-result
|
||||
(if value
|
||||
;; An expression's value, rendered inside the running program —
|
||||
@ -195,18 +289,14 @@ With no argument, look for `flan-dev-socket-name' up from this buffer."
|
||||
(if fns (string-join fns ", ")
|
||||
(if names (string-join names ", ") what))
|
||||
(or (plist-get reply :ms) 0))))))
|
||||
;; The daemon reports where, so put point there when it is this buffer.
|
||||
;; The daemon reports where, so mark it there. This must not itself
|
||||
;; signal: the error the caller is owed is the daemon's, and losing it to a
|
||||
;; bad location would report the wrong thing entirely.
|
||||
(let ((loc (plist-get reply :loc))
|
||||
(msg (plist-get reply :message)))
|
||||
(when (and loc (string-match "\\`\\(.*\\):\\([0-9]+\\):\\([0-9]+\\)\\'" loc))
|
||||
(let ((file (match-string 1 loc))
|
||||
(line (string-to-number (match-string 2 loc)))
|
||||
(col (string-to-number (match-string 3 loc))))
|
||||
(when (and buffer-file-name (file-equal-p file buffer-file-name))
|
||||
(goto-char (point-min))
|
||||
(forward-line (1- line))
|
||||
(forward-char (max 0 (1- col))))))
|
||||
(user-error "flan: %s" (or msg "rejected")))))
|
||||
(ignore-errors (flan-dev--show-error loc (or msg "rejected")))
|
||||
(user-error "flan: %s%s" (or msg "rejected")
|
||||
(if loc (format " (%s)" loc) "")))))
|
||||
|
||||
(defun flan-dev--eval (code what)
|
||||
"Send CODE to the running program. WHAT names it for the echo area."
|
||||
@ -217,19 +307,37 @@ With no argument, look for `flan-dev-socket-name' up from this buffer."
|
||||
(list :op "eval" :code code :file (or buffer-file-name "<buffer>")))
|
||||
what))
|
||||
|
||||
(defun flan-dev--defun-at-point ()
|
||||
"The text of the top-level form containing or preceding point."
|
||||
(defun flan-dev--text (start end)
|
||||
"The buffer text from START to END, on the line it is actually written on.
|
||||
|
||||
The daemon reads what it is sent starting at line 1, so a form taken from the
|
||||
middle of a buffer comes back with a location relative to the *snippet* — and
|
||||
an error overlay drawn from that sits on line 1 of the file, pointing at
|
||||
whatever happens to be there. Leading newlines are the whole fix: the reader
|
||||
skips them, and the line numbers in the reply are then the buffer's own. The
|
||||
columns already were, because a top-level form starts at column 1."
|
||||
(concat (make-string (1- (line-number-at-pos start)) ?\n)
|
||||
(buffer-substring-no-properties start end)))
|
||||
|
||||
(defun flan-dev--defun-bounds ()
|
||||
"Bounds of the top-level form containing or preceding point, as (START . END)."
|
||||
(save-excursion
|
||||
(end-of-defun)
|
||||
(let ((end (point)))
|
||||
(beginning-of-defun)
|
||||
(buffer-substring-no-properties (point) end))))
|
||||
(cons (point) end))))
|
||||
|
||||
(defun flan-dev--defun-at-point ()
|
||||
"The text of the top-level form containing or preceding point."
|
||||
(let ((b (flan-dev--defun-bounds)))
|
||||
(buffer-substring-no-properties (car b) (cdr b))))
|
||||
|
||||
;;;###autoload
|
||||
(defun flan-eval-defun ()
|
||||
"Recompile the top-level form at point and install it in the running program."
|
||||
(interactive)
|
||||
(flan-dev--eval (flan-dev--defun-at-point) "form"))
|
||||
(let ((b (flan-dev--defun-bounds)))
|
||||
(flan-dev--eval (flan-dev--text (car b) (cdr b)) "form")))
|
||||
|
||||
;;;###autoload
|
||||
(defun flan-eval-buffer ()
|
||||
@ -256,7 +364,7 @@ arrive in the same load or the first refers to storage that does not exist."
|
||||
(defun flan-eval-region (start end)
|
||||
"Recompile the top-level forms between START and END."
|
||||
(interactive "r")
|
||||
(flan-dev--eval (buffer-substring-no-properties start end) "region"))
|
||||
(flan-dev--eval (flan-dev--text start end) "region"))
|
||||
|
||||
(provide 'flan-dev)
|
||||
;;; flan-dev.el ends here
|
||||
|
||||
@ -63,6 +63,65 @@
|
||||
(test-flan--check "a form that does not check is reported"
|
||||
(and raised (string-match-p "unknown name" raised))))
|
||||
|
||||
;; The :loc column is a *byte* offset — lib/reader.ml walks the source a byte
|
||||
;; at a time — and Emacs counts characters. Same rule as the framing, a
|
||||
;; different place to get it wrong, and it shows up only for someone whose
|
||||
;; comments or identifiers are not ASCII.
|
||||
(with-temp-buffer
|
||||
(insert ";; héllo\n(defn wörld [] i64 nonsense)\n")
|
||||
(let ((want (save-excursion
|
||||
(goto-char (point-min))
|
||||
(search-forward "nonsense")
|
||||
(match-beginning 0)))
|
||||
(eol (save-excursion (goto-char (point-min)) (line-end-position))))
|
||||
(test-flan--check "a byte column lands on the right character"
|
||||
(= (flan-dev--position
|
||||
2 (1+ (string-bytes "(defn wörld [] i64 ")))
|
||||
want))
|
||||
(test-flan--check "a column past the end of a line is clamped to it"
|
||||
(= (flan-dev--position 1 500) eol))))
|
||||
|
||||
;; A rejected form is marked where it is, not only in the echo area. The
|
||||
;; daemon numbers lines from the start of what it was sent, so a form taken
|
||||
;; from the middle of a buffer only lands on the right line because the
|
||||
;; client pads it back into place before sending.
|
||||
(goto-char (point-min))
|
||||
(search-forward "(defn step")
|
||||
(goto-char (match-beginning 0))
|
||||
(let ((defn-line (line-number-at-pos)))
|
||||
(save-excursion
|
||||
(search-forward "(+ ticks 41)")
|
||||
(replace-match "(+ ticks nonsense)"))
|
||||
(ignore-errors (flan-eval-defun))
|
||||
(let ((ovs (seq-filter (lambda (o) (overlay-get o 'flan-dev-error))
|
||||
(overlays-in (point-min) (point-max)))))
|
||||
(test-flan--check "a rejected form gets exactly one error overlay"
|
||||
(= 1 (length ovs)))
|
||||
(test-flan--check "the overlay is at the form, not at line 1"
|
||||
(and ovs (>= (line-number-at-pos (overlay-start (car ovs)))
|
||||
defn-line)))
|
||||
(test-flan--check "the overlay carries the daemon's reason"
|
||||
(and ovs
|
||||
(string-match-p
|
||||
"unknown name"
|
||||
(or (overlay-get (car ovs) 'help-echo) ""))))
|
||||
(test-flan--check "and shows it beside the code"
|
||||
(and ovs
|
||||
(string-match-p
|
||||
"unknown name"
|
||||
(or (overlay-get (car ovs) 'after-string) "")))))
|
||||
;; ...and it goes away when the next evaluation is accepted. A marker left
|
||||
;; behind after a fix is a lie about the running program. Point moved to
|
||||
;; the error, which is the point of all this, so start the search over.
|
||||
(goto-char (point-min))
|
||||
(search-forward "(+ ticks nonsense)")
|
||||
(replace-match "(+ ticks 41)")
|
||||
(search-backward "(defn step")
|
||||
(flan-eval-defun)
|
||||
(test-flan--check "an accepted evaluation clears it"
|
||||
(null (seq-filter (lambda (o) (overlay-get o 'flan-dev-error))
|
||||
(overlays-in (point-min) (point-max))))))
|
||||
|
||||
;; The session is not poisoned by that: a good form still lands.
|
||||
(flan-dev--eval "(defn step [] i64 (set ticks (+ ticks 100)) ticks)" "form")
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user