alphapapa / prism.el

Disperse Lisp forms (and other languages) into a spectrum of colors by depth
GNU General Public License v3.0
285 stars 4 forks source link

prism-whitespace-mode: "End of buffer" when used with swiper, org-export, etc. #6

Closed vuori closed 1 year ago

vuori commented 4 years ago

Prism seems to have a weird interaction with Swiper at least in prism-whitespace-mode: when Swiper is invoked with Prism enabled, sometimes Emacs will just beep and show "End of buffer" in the minibuffer. Trying again will usually work. With Prism disabled, Swiper always works.

On Emacs 27.0.90, I can repeat it like this:

  1. Start emacs -Q and evaluate:
    (package-initialize)
    (require 'prism)
    (require 'swiper)
  2. Open a file in python-mode. (2b. Try M-x swiper, note that it works.)
  3. Invoke M-x prism-whitespace-mode.
  4. Try M-x swiper, note that it dies with "End of buffer". (4b. Try M-x swiper again, note that it works most of the time.)

I'm not sure if this is a bug in Prism, Swiper or Emacs. The End of buffer error doesn't seem to trigger debug-on-error. Any ideas on debugging this?

alphapapa commented 4 years ago

You could try using debug-on-message.

Without tracing the code, it's hard to say whether it's caused by Prism or Swiper. I'm guessing that what's happening is, Swiper is causing fontification of lines in the buffer that have yet to be fontified; calling Swiper a second time (with the same input, that is) doesn't cause the message, because all the lines it will display have been fontified.

But that doesn't explain the message, or why it would cause Swiper to fail. The same basic behavior should be the case using similar tools, like some Helm commands. E.g. I use this simple Helm command (https://gist.github.com/alphapapa/ce6cf8a85d8a28c3a2f146057772ec8d), but I don't have the problem you describe.

vuori commented 4 years ago

Looks like debug-on-message doesn't catch the "End of buffer" for whatever reason. However, after some digging, it seems that it's coming from font-lock-ensure.

So simplified (again on recent Emacs 27.0.90 build):

  1. Start emacs -Q and evaluate:
    (package-initialize)
    (require 'prism)
  2. Open a file in python-mode.
  3. Invoke M-x prism-whitespace-mode.
  4. Eval (font-lock-ensure), note that it dies with "End of buffer".
alphapapa commented 4 years ago

Looks like debug-on-message doesn't catch the "End of buffer" for whatever reason.

That reason might be significant.

However, after some digging, it seems that it's coming from font-lock-ensure.

I'm not an expert on the font-lock system, but I'm guessing that the message is caused by code in Prism which is activated by calling font-lock-ensure. It's probably in prism-extend-region or prism-match-whitespace, which move the point around.

Eval (font-lock-ensure), note that it dies with "End of buffer".

There may be a misunderstanding or unstated assumption here. "Dies" is not a technical term. ;) Calling font-lock-ensure in the recipe you provided appears to properly fontify the whole buffer, so I'm not sure if the message indicates a problem or a bug in Prism.

Since you filed this issue due to a problem with Swiper, I'm not sure if this indicates a problem with Prism or with Swiper. Maybe Prism is moving point in a way that causes an "error" (i.e. the kind of error that is signaled if you do C-f with point at point-max, which isn't necessarily an "error"), and if so, maybe that should be fixed, if possible. But maybe Swiper should be able to cope with those kinds of "errors."

vuori commented 4 years ago

The only occasion I've seen spontaneous "End of buffer" errors/messages is with prism + ensure-font-lock, but I'm not really familiar with elisp exception handling so all I can say here is 🤷‍♀️

For Google-posterity, here is a workaround for init.el that will make swiper skip font-lock calls (I haven't noticed any significant problems elsewhere):

(defun swiper-font-lock-ensure-p ()
    t)
alphapapa commented 4 years ago

I'm guessing that, regardless of whether this indicates a minor problem in Prism, Swiper ought not abort or fail when this happens. Maybe you should file a bug on the Swiper repo and see what abo-abo thinks.

Here's probably a better workaround:

(define-advice swiper-font-lock-ensure (:around (oldfun))
  (ignore-errors
    (funcall oldfun)))

BTW, note that Swiper may be much slower than alternatives. In my config, using Swiper on my 13k-line scratch buffer takes a few seconds to activate, while e.g. the helm-swish command I linked activates nearly instantly.

vuori commented 4 years ago

Thanks. swiper's issue tracker looked like there were quite a few uncommented open issues a while ago, but I'll keep an eye on it at least. I liked swiper's highlighting of the search term and ivy interface when bound to C-s (I use helm-swiper with another binding occasionally when I want to see a lot of matches, not just the nearby ones), but swish's more compact codebase and speed certainly looks like a plus.

alphapapa commented 4 years ago

I manually instrumented prism-match-whitespace and called font-lock-ensure, but I can't find what's causing the End of buffer message. It seems like End of buffer shouldn't be happening, which suggests it needs to be fixed in Prism, but I don't know why it's happening. So I still don't know if it's a bug in Prism, or if Swiper should be more "tolerant" of whatever's going on here.

To compare, I added a call to font-lock-ensure to the helm-swish-init function in the gist I linked, and it seems to work without interrupting the Helm session. So regardless of whether there's a bug in Prism here, Swiper should probably be able to cope with it.

For future reference, here's the prism-match-whitespace function with a bunch of message calls to try to locate the source of the message. In the output, when calling font-lock-ensure, the last message printed is the after (goto-char ... one.

(defun prism-match-whitespace (limit)
  "Matcher function for `font-lock-keywords' in whitespace-sensitive buffers.
Matches up to LIMIT.  Requires `prism-whitespace-indent-offset' be set
appropriately, e.g. to `python-indent-offset' for `python-mode'."
  (cl-macrolet ((parse-syntax ()
                              `(-setq (list-depth _ _ in-string-p comment-level-p _ _ _ comment-or-string-start)
                                 (syntax-ppss)))
                (indent-depth ()
                              `(cond ((save-excursion
                                        (message "forward-line -1")
                                        (forward-line -1)

                                        (unless (eobp)
                                          (message "(looking-at-p (rx (1+ nonl) eol))")
                                          (looking-at-p (rx (1+ nonl) "\\" eol))))
                                      (progn
                                        (message "(/ (current-indentation) (* 2 prism-whitespace-indent-offset))")
                                        (/ (current-indentation) (* 2 prism-whitespace-indent-offset))))
                                     (t (progn
                                          (message "(/ (current-indentation) prism-whitespace-indent-offset)")
                                          (/ (current-indentation) prism-whitespace-indent-offset)))))
                (depth-at ()
                          ;; Yes, this is entirely too complicated--just like Python's syntax in
                          ;; comparison to Lisp.  But, "Eww, all those parentheses!"  they say.
                          ;; Well, all those parentheses avoid lots of special cases like these.
                          `(pcase list-depth
                             (0 (cond ((progn
                                         (message "(looking-at-p (rx (syntax close-parenthesis) eol))")
                                         (looking-at-p (rx (syntax close-parenthesis) eol)))
                                       (save-excursion
                                         (message "forward-char 1")
                                         (forward-char 1)
                                         (backward-sexp 1)
                                         (+ (nth 0 (syntax-ppss)) (indent-depth))))
                                      ((progn
                                         (message "(looking-back (rx (syntax close-parenthesis)) (1- (point)))")
                                         (looking-back (rx (syntax close-parenthesis)) (1- (point))))
                                       (save-excursion
                                         (backward-sexp 1)
                                         (+ (nth 0 (syntax-ppss)) (indent-depth))))
                                      (t (indent-depth))))
                             (_ (save-excursion
                                  ;; Exit lists back to depth 0.
                                  (message "(goto-char (scan-lists (point) -1 (nth 0 (syntax-ppss))))")
                                  (goto-char (scan-lists (point) -1 (nth 0 (syntax-ppss))))
                                  (+ list-depth (indent-depth))))))
                (comment-p ()
                           ;; This macro should only be used after `parse-syntax'.
                           `(or comment-level-p (looking-at-p (rx (or (syntax comment-start)
                                                                      (syntax comment-delimiter))))))
                (face-at ()
                         ;; Return face to apply.  Should be called with point at `start'.
                         `(let ((depth (depth-at)))
                            (cond ((comment-p)
                                   (pcase depth
                                     (0 'font-lock-comment-face)
                                     (_ (if prism-faces-comments
                                            (alist-get depth prism-faces-comments)
                                          (alist-get depth prism-faces)))))
                                  ((or in-string-p (looking-at-p (rx (or (syntax string-quote)
                                                                         (syntax string-delimiter)))))
                                   (pcase depth
                                     (0 'font-lock-string-face)
                                     (_ (if prism-faces-strings
                                            (alist-get depth prism-faces-strings)
                                          (alist-get depth prism-faces)))))
                                  (t (alist-get depth prism-faces))))))
    (message "entered prism-match-whitespace")
    (with-syntax-table prism-syntax-table
      (unless (eobp)
        ;; Not at end-of-buffer: start matching.
        (message "not eobp")
        (let ((parse-sexp-ignore-comments t)
              list-depth in-string-p comment-level-p comment-or-string-start start end
              found-comment-p found-string-p)
          (while ;; Skip to start of where we should match.
              (and (not (eobp))
                   (cond ((eolp)
                          (unless (eobp)
                            (message "forward-line 1")
                            (forward-line 1)))
                         ((looking-at-p (rx blank))
                          (message "(forward-whitespace 1)")
                          (forward-whitespace 1))
                         ((unless prism-strings
                            (when (looking-at-p (rx (syntax string-quote)))
                              ;; At a string: skip it.
                              (message "(forward-sexp)")
                              (forward-sexp))))
                         ((unless prism-comments
                            (message "(forward-comment (buffer-size))")
                            (forward-comment (buffer-size)))))))
          (parse-syntax)
          (when in-string-p
            ;; In a string: go back to its beginning (before its delimiter).
            ;; It would be nice to leave this out and rely on the check in
            ;; the `while' above, but if partial fontification starts inside
            ;; a string, we have to handle that.
            ;; NOTE: If a string contains a Lisp comment (e.g. in
            ;; `custom-save-variables'), `in-string-p' will be non-nil, but
            ;; `comment-or-string-start' will be nil.  I don't know if this
            ;; is a bug in `parse-partial-sexp', but we have to handle it.
            (when comment-or-string-start
              (message "(goto-char comment-or-string-start)")
              (goto-char comment-or-string-start)
              (unless prism-strings
                (message "(forward-sexp)")
                (forward-sexp))
              (parse-syntax)))
          ;; Set start and end positions.
          (setf start (point)
                ;; I don't know if `ignore-errors' is going to be slow, but since
                ;; `scan-lists' and `scan-sexps' signal errors, it seems necessary if we want
                ;; to use them (and they seem to be cleaner to use than regexp searches).
                end (min limit
                         (save-excursion
                           (or (when (and prism-comments (comment-p))
                                 (setf found-comment-p t)
                                 (when comment-or-string-start
                                   (message "(goto-char comment-or-string-start)")
                                   (goto-char comment-or-string-start))
                                 ;; We must only skip one comment, because before there is
                                 ;; non-comment, non-whitespace text, the indent depth might change.
                                 (message "(forward-comment 1)")
                                 (forward-comment 1)
                                 (point))
                               (when (looking-at-p (rx (syntax close-parenthesis)))
                                 ;; I'd like to just use `scan-lists', but I can't find a way around this initial check.
                                 ;; The code (scan-lists start 1 1), when called just inside a list, scans past the end
                                 ;; of it, to just outside it, which is not what we want, because we want to highlight
                                 ;; the closing paren with the shallower depth.  But if we just back up one character,
                                 ;; we never exit the list.  So we have to check whether we're looking at the close of a
                                 ;; list, and if so, move just past it.
                                 (cl-decf list-depth)
                                 (1+ start))
                               (when (looking-at-p (rx (or (syntax string-quote)
                                                           (syntax string-delimiter))))
                                 (message "(forward-sexp 1)")
                                 (forward-sexp 1)
                                 (setf found-string-p t)
                                 (point))
                               ;; Don't go past the end of the line.
                               (apply #'min
                                      (-non-nil
                                       (list
                                        (or (ignore-errors
                                              (message "(scan-lists start 1 -1)")
                                              ;; Scan to the past the delimiter of the next deeper list.
                                              (scan-lists start 1 -1))
                                            (ignore-errors
                                              (message "(1- (scan-lists start 1 1))")
                                              ;; Scan to the end of the current list delimiter.
                                              (1- (scan-lists start 1 1))))
                                        (line-end-position))))
                               ;; If we can't find anything, return `limit'.  I'm not sure if this is the correct
                               ;; thing to do, but it avoids an error (and possibly hanging Emacs) in the event of
                               ;; an undiscovered bug.  Although, signaling an error might be better, because I
                               ;; have seen "redisplay" errors related to font-lock in the messages buffer before,
                               ;; which might mean that Emacs can handle that.  I think the important thing is not
                               ;; to hang Emacs, to always either return nil or advance point to `limit'.
                               limit))))
          (when (< end start)
            ;; Set search bound properly when `start' is greater than
            ;; `end' (i.e. when `start' is moved past `limit', I think).
            (setf end start))
          (when end
            ;; End found: Try to fontify.
            (unless (or in-string-p found-string-p found-comment-p)
              ;; Neither in a string nor looking at nor in a comment.
              (save-excursion
                (or (when (re-search-forward (rx (syntax comment-start)) end t)
                      ;; Set `end' to any comment found before it.
                      (setf end (match-beginning 0)))
                    (when (re-search-forward (rx (or (syntax string-quote)
                                                     (syntax string-delimiter)))
                                             end t)
                      ;; Set `end' to any string found before it.
                      (unless (nth 4 (syntax-ppss))
                        ;; Not in a comment.
                        (setf end (match-beginning 0)))))))
            (if (and (comment-p) (= 0 (depth-at)))
                (setf prism-face nil)
              (setf prism-face (face-at)))
            (message "(goto-char end:%s)" end)
            (goto-char end)
            (message "after (goto-char end:%s)" end)
            (set-match-data (list start end (current-buffer)))
            ;; Be sure to return non-nil!
            t))))))
piknik commented 1 year ago

I'm running Emacs 28.2.

I load my init.el without prism-mode to use consult-line and it's fine. Once I toggle prism-mode on in the same buffer and session in the init.el, the error appears. Once the buffer is exited and then entered into again, the error may be reproduced.

This code allows the end-of-buffer error to be backtraced:

  (setf debug-ignored-errors (seq-filter (lambda (elt) (not (eq 'end-of-buffer elt)))
                     debug-ignored-errors)
    debug-on-error t)

Then the backtrace may be viewed:

  Debugger entered--Lisp error: (end-of-buffer)
    font-lock-fontify-keywords-region(1503 30761 nil)
    font-lock-default-fontify-region(1503 30761 nil)
    font-lock-fontify-region(1503 30761)
    #f(compiled-function (fun) #<bytecode 0x19b6850ba430613d>)(font-lock-fontify-region)
    jit-lock--run-functions(1503 30761)
    jit-lock-fontify-now()
    consult--fontify-all()
    consult--line-candidates(nil 1)
    consult-line(nil nil)
    funcall-interactively(consult-line nil nil)
    command-execute(consult-line)

After re-defining the offending function (C-x C-e), the backtrace may be viewed with more detail:

  Debugger entered--Lisp error: (end-of-buffer)
    forward-char(1)
    (progn (forward-char 1) t)
    (or (> (point) (match-beginning 0)) (progn (forward-char 1) t))
    (and (< (point) end) (if (stringp matcher) (re-search-forward matcher end t) (funcall matcher end)) (or (> (point) (match-beginning 0)) (progn (forward-char 1) t)))
    (while (and (< (point) end) (if (stringp matcher) (re-search-forward matcher end t) (funcall matcher end)) (or (> (point) (match-beginning 0)) (progn (forward-char 1) t))) (if (and font-lock-multiline (>= (point) (save-excursion (goto-char (match-beginning 0)) (forward-line 1) (point)))) (progn (put-text-property (if (= (point) (save-excursion (goto-char ...) (forward-line 1) (point))) (1- (point)) (match-beginning 0)) (point) 'font-lock-multiline t))) (setq highlights (cdr keyword)) (while highlights (if (numberp (car (car highlights))) (font-lock-apply-highlight (car highlights)) (set-marker pos (point)) (font-lock-fontify-anchored-keywords (car highlights) end) (if (< (point) pos) (goto-char pos))) (setq highlights (cdr highlights))))
    (while keywords (if loudly (message "Fontifying %s... (regexps..%s)" bufname (make-string (setq count (1+ count)) 46))) (setq keyword (car keywords) matcher (car keyword)) (goto-char start) (while (and (< (point) end) (if (stringp matcher) (re-search-forward matcher end t) (funcall matcher end)) (or (> (point) (match-beginning 0)) (progn (forward-char 1) t))) (if (and font-lock-multiline (>= (point) (save-excursion (goto-char (match-beginning 0)) (forward-line 1) (point)))) (progn (put-text-property (if (= (point) (save-excursion ... ... ...)) (1- (point)) (match-beginning 0)) (point) 'font-lock-multiline t))) (setq highlights (cdr keyword)) (while highlights (if (numberp (car (car highlights))) (font-lock-apply-highlight (car highlights)) (set-marker pos (point)) (font-lock-fontify-anchored-keywords (car highlights) end) (if (< (point) pos) (goto-char pos))) (setq highlights (cdr highlights)))) (setq keywords (cdr keywords)))
    (let ((case-fold-search font-lock-keywords-case-fold-search) (keywords (cdr (cdr font-lock-keywords))) (bufname (buffer-name)) (count 0) (pos (make-marker)) keyword matcher highlights) (while keywords (if loudly (message "Fontifying %s... (regexps..%s)" bufname (make-string (setq count (1+ count)) 46))) (setq keyword (car keywords) matcher (car keyword)) (goto-char start) (while (and (< (point) end) (if (stringp matcher) (re-search-forward matcher end t) (funcall matcher end)) (or (> (point) (match-beginning 0)) (progn (forward-char 1) t))) (if (and font-lock-multiline (>= (point) (save-excursion (goto-char ...) (forward-line 1) (point)))) (progn (put-text-property (if (= ... ...) (1- ...) (match-beginning 0)) (point) 'font-lock-multiline t))) (setq highlights (cdr keyword)) (while highlights (if (numberp (car (car highlights))) (font-lock-apply-highlight (car highlights)) (set-marker pos (point)) (font-lock-fontify-anchored-keywords (car highlights) end) (if (< (point) pos) (goto-char pos))) (setq highlights (cdr highlights)))) (setq keywords (cdr keywords))) (set-marker pos nil))
    font-lock-fontify-keywords-region(1503 30761 nil)
    font-lock-default-fontify-region(1503 30761 nil)
    font-lock-fontify-region(1503 30761)
    #f(compiled-function (fun) #<bytecode 0x19b6850ba430613d>)(font-lock-fontify-region)
    jit-lock--run-functions(1503 30761)
    jit-lock-fontify-now()
    consult--fontify-all()
    consult--line-candidates(nil 1)
    consult-line(nil nil)
    funcall-interactively(consult-line nil nil)
    command-execute(consult-line)
alphapapa commented 1 year ago

@piknik Yes, I've seen something similar happening lately when using consult-line. AFAIK it's only happening with prism-whitespace-mode, not with prism-mode (regrettably, it doesn't seem feasible to combine the two functions; maybe in the future, a tree-sitter-based implementation could be simpler).

I intend to fix this when I have time, but it may not be soon. Thanks.

alphapapa commented 1 year ago

I've actually noticed this happening when using Org Export on some of my readmes containing Elisp source blocks. Haven't been able to find where in the code the error is happening, though. sigh

alphapapa commented 1 year ago

@vuori @piknik I think I finally solved this problem. It was difficult, requiring me to dig into some font-lock functions to reproduce the problem. I still don't understand why it happened, and I can't say whether this is the "correct" solution, but this seems to fix it. Please let me know if it works for you. Thanks.