Understanding sly-mrepl--syntax-propertize

[dieggsy]

I was playing around with adding syntax highlighting in sly-mrepl-mode and copying font-lock-defaults from lisp-mode almost got me there:

(add-hook 'sly-mrepl-mode-hook
          (lambda ()
            (setq font-lock-defaults
                  '((lisp-cl-font-lock-keywords lisp-cl-font-lock-keywords-1 lisp-cl-font-lock-keywords-2)
                    nil t nil nil
                    (font-lock-mark-block-function . mark-defun)
                    (font-lock-extra-managed-props help-echo)
                    (font-lock-syntactic-face-function . lisp-font-lock-syntactic-face-function)))))

But I was seeing odd behavior - everything but the current input syntax highlighted as a comment. I found that the culprit was sly-mrepl--syntax-propertize, which I did a quick and dirty override for:

(define-advice sly-mrepl--syntax-propertize (:override (beg end) d/no-comment-syntax)
  "Advice to avoid funky syntax stuff that sly-mrepl does for
unclear reasons. Maybe I will break everything down the line!
Exciting."
  nil)

This appears to do what I want, at least on the surface, but I'm concerned that there's a good reason not to obliterate this function. The docstring says Make everything up to current prompt comment syntax. What is the purpose of this?

[joaotavora]

As far as I can remember, this function makes everything up to current prompt comment syntax (as the docstring says), to turn off parenthesis matching in those parts. That's so that, when using things like M-x electric-pair-mode and other parenthesis matchers, they only consider the parenthesis of the current input prompt, and not any old output that may not having matching parenthesis.

[dieggsy]

Interesting. Maybe this should be applied at a per output level instead of on the whole buffer? I'd guess there's some better way to implement syntax highlighting to coexist with this, but I'm not too worried about overriding it for now.

[joaotavora]

It's not just the output, may also be some faulty input that doesn't balance parenthesis.

But you may argue that making the entire thing comment syntax is overkill . You could just tweak the syntax of those things
that are problematic. But that's lot of work. And comment syntax has other benefits.

What syntax highlighting exactly are planning to implement?

[dieggsy]

@joaotavora just common lisp syntax highlighting for input

[joaotavora]

Hmmm, so if it's just for the input, what's the trouble with making all the other non-input parts a certain syntax?

[dieggsy]

Probably just that I'm implementing it incorrectly 😅. I'm not sure how to restrict font locking to just inputs, so I was using the rules for common lisp buffers, which I admit is probably not actually what I want.

[dieggsy]

Well, but I'd like to preserve syntax highlighting on previous inputs as well

[joaotavora]

Well, but I'd like to preserve syntax highlighting on previous inputs as well

In that case, I think you could "burn" the previously calculated properties with font-lock-face in those regions. But I'd focus on getting font-lock to work correctly in input text first.

[dieggsy]

Ok, I'm fairly certain I have something that's properly working now, with no overriding of sly-mrepl-syntax-propertize:

;;; -*- lexical-binding: t -*-
(defun make-sly-font-lock-matcher (matcher)
  (lambda (limit)
     ;; matcher can be a function or a regex
     (when (if (stringp matcher)
               (re-search-forward matcher limit t)
             (funcall matcher limit))
       ;; while the subexp can be anything, this search always can use zero
       (let ((start (match-beginning 0)))
         (and (or (eql 'sly-mrepl-input (get-text-property start 'field))
                  (get-text-property start 'sly-mrepl--prompt)))))))

(defun font-lock-keyword-to-sly-mrepl-keyword (kwd)
  "converts a `font-lock-keywords' kwd for `comint-mode' input fontification."
  (cl-destructuring-bind (matcher . match-highlights) kwd
    `(,(make-sly-font-lock-matcher matcher) ,@match-highlights)))

(defun d/sly-mrepl-font-lock-syntactic-face-function (state)
  (let ((start (nth 8 state)))
    (and (not (member (get-text-property start 'field) '(output sly-mrepl--output)))
         (lisp-font-lock-syntactic-face-function state))))

(defvar d/sly-mrepl-font-lock-keywords
  `(,@(mapcar #'font-lock-keyword-to-sly-mrepl-keyword lisp-cl-font-lock-keywords)
    ,@(mapcar #'font-lock-keyword-to-sly-mrepl-keyword lisp-cl-font-lock-keywords-1)
    ,@(mapcar #'font-lock-keyword-to-sly-mrepl-keyword lisp-cl-font-lock-keywords-2)))

(add-hook 'sly-mrepl-mode-hook
          (lambda ()
            (setq font-lock-defaults
                  `(d/sly-mrepl-font-lock-keywords
                    nil t nil nil
                    (font-lock-mark-block-function . mark-defun)
                    (font-lock-extra-managed-props help-echo)
                    (font-lock-syntactic-face-function . d/sly-mrepl-font-lock-syntactic-face-function)))))

I'm converting lisp's font lock matchers (regexp's or functions) into a function that first checks we're in a sly prompt/input and then proceeds with fontifying.

This works reasonably well and highlighting from these rules is preserved, but syntactic font-locking (comments, strings, rainbow-delimiters) is (expectedly) not preserved once things get changed to comment syntax.

Not sure I follow how you were suggesting burning calculated properties?

EDIT: Whoops, updated end of code snippet

[monnier]

;; matcher can be a function or a regex (when ,(if (or (symbolp matcher) (functionp matcher)) `(funcall ',matcher limit) `(re-search-forward ,matcher limit t))

Side note: font-lock tests `stringp` instead (which is simpler and more efficient).

;; while the subexp can be anything, this search always can use zero (let ((start (match-beginning 0)) (state (syntax-ppss))) (and (or (eql 'sly-mrepl-input (get-text-property start 'field)) (get-text-property start 'sly-mrepl--prompt)))))))

You don't use `state` here. Stefan

[dieggsy]

@monnier thanks for the notes! I did modify and iterate on someone else's code and forgot I stopped using state (intentionally). I have cleaned up the code block in the original comment.

[joaotavora]

Another note:

(defun make-sly-font-lock-matcher (matcher)
  `(lambda (limit)

Seems a bit strange to be using backquoted lambda. Are you sure you don't want to just use lexical binding?

[dieggsy]

Good call. I do in fact want to use lexical binding. Updated once more. Any thoughts on preserving comment/string highlighting in previous inputs? If there was a function that froze syntactic font locking as text properties or something like that...

Oh, syntactic works with text properties anyway, of course. I'd just like to keep the face around after entering the input I guess.