modes derived from logview cannot refine font-lock

[Apteryks]

Hi,

I'm trying to come up with a derived mode. This currently corresponds of a logview submode configuration (which could be integrated here) but also of additional font-lock keywords and extra navigation code (future work). It currently looks like this:

;;; Commentary:

;; This file provides a major mode (`robot-log-mode') for highlighting
;; RobotFramework debug logs.  This mode was inspired by
;; `guix-build-log' from the `emacs-guix' package; thanks!

;;; Code:

(require 'logview)

;;; TODO
;;; - Foldable hierarchy + select functions (root, level 1, 2, etc.)


;;;
;;; Regexps.
;;;

(defvar robot-log-keyword-start-regexp
  "\\+-* START KEYWORD: \\(.*\\) \\[\\(.*\\)\\]"
  "Regexp for the start line of a keyword.")

(defvar robot-log-keyword-end-regexp
  "\\+-* END KEYWORD: \\(.*\\) (\\(.*\\))"
  "Regexp for the ending line of a keyword.")

(defvar robot-log-imenu-generic-expression
  `((nil robot-log-keyword-start-regexp 1))
  "Imenu generic expression for `robot-log-mode'.")


;;;
;;; Font lock configuration.
;;;

(defface robot-log-keyword-start
  '((default :weight bold))
  "Face for RobotFramework keywords (start).")

(defface robot-log-keyword-arguments
  '((default :foreground "LightBlue"))
  "Face for RobotFramework keyword arguments.")

(defface robot-log-keyword-end
  '((default :weight bold))
  "Face for RobotFramework keywords (end).")

(defface robot-log-keyword-exit-status
  '((default :foreground "LightGreen"))
  "Face for RobotFramework keyword exit statuses.")

(defvar robot-log-font-lock-keywords
  `((,robot-log-keyword-start-regexp
     (1 'robot-log-keyword-start)
     (2 'robot-log-keyword-arguments))
    (,robot-log-keyword-end-regexp
     (1 'robot-log-keyword-end)
     (2 'robot-log-keyword-exit-status)))
  "A list of `font-lock-keywords' for `robot-log-mode'.")


;;;
;;; Navigation.
;;;

;;; TODO
(defvar robot-log-common-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "SPC") (lambda () (message "test\n"))  ;'robot-log-next-keyword
      )
    ;; (define-key map (kbd "M-p") 'robot-log-previous-keyword)
    ;; (define-key map (kbd "TAB") 'robot-log-keyword-toggle)
    ;; (define-key map (kbd "<tab>") 'robot-log-keyword-toggle)
    ;; (define-key map (kbd "<backtab>") 'robot-log-toggle-all)
    ;; (define-key map [(shift tab)] 'robot-log-toggle-all)
    map)
  "Parent keymap for 'robot-log' buffers, adding or overriding
extra keys on top of those of `logview-mode'.")

(defvar robot-log-mode-map
  (let ((map (make-sparse-keymap)))
    (set-keymap-parent map (make-composed-keymap
                            (list robot-log-common-map)
                            logview-mode-map))
    map)
  "Keymap for `robot-log-mode' buffers.")

;;; TODO: Contribute upstream.  This needs to be at the top level so
;;; that the parent logview-mode knows its definition before it is
;;; initialized.
(add-to-list 'logview-additional-level-mappings
             '("RobotFramework"
               (error       "FAIL")
           (warning     "WARNING")
           (information "INFO" "HTML")
           (debug       "DEBUG")
           (trace       "TRACE")
           (aliases "RF" "Robot")))

(add-to-list 'logview-additional-timestamp-formats
             '("RobotFramework"
               (java-pattern . "yyyyMMdd HH:mm:ss.SSS")
               (datetime-options :any-decimal-separator t)
               (aliases "Robot" "RF")))

(add-to-list 'logview-additional-submodes
             '("RobotFramework"
               (format . "TIMESTAMP - LEVEL - MESSAGE")
               (levels . "RobotFramework")
               (timestamp "RobotFramework")
               (aliases "RF" "Robot")))

;;;###autoload
(define-derived-mode robot-log-mode logview-mode
  "Robot-Log"
  "Major mode for viewing RobotFramework debug logs.

\\{robot-log-mode-map}"
  ;;(font-lock-add-keywords nil robot-log-font-lock-keywords t)

  (setq font-lock-defaults '(robot-log-font-lock-keywords t)
        imenu-generic-expression robot-log-imenu-generic-expression))

(provide 'robot-log)

The problem I'm having is that my font-lock-defauts are not honored. After investigating and reading, this seems to be caused by logview directly overriding the internal font-lock-fontify-region-function procedure. In info 'elisp (Font Lock Basics)', it says:

There are several variables that control how Font Lock mode highlights text. But major modes should not set any of these variables directly. Instead, they should set ‘font-lock-defaults’ as a buffer-local variable. The value assigned to this variable is used, if and when Font Lock mode is enabled, to set all the other variables.

font-lock-defaults can be provided with OTHER-VARS, which includes font-lock-fontify-region-function, so I think logview should use font-lock-defaults to bind #'logview--fontify-region to font-lock-fontify-region-function. Do you think that'd help with my issue?

Thank you for this useful mode!

[Apteryks]

I've edited logview.el a bit like so (note: font-lock-defaults):

(define-derived-mode logview-mode nil "Logview"
  "Major mode for viewing and filtering various log files."
  (logview--update-keymap)
  (add-hook 'read-only-mode-hook #'logview--update-keymap nil t)
  (setq font-lock-defaults
        `(nil nil nil nil
              (font-lock-fontify-region-function . ,'logview--fontify-region)))
  ;; (set (make-local-variable 'font-lock-fontify-region-function) #'logview--fontify-region)
  (set (make-local-variable 'filter-buffer-substring-function)  #'logview--buffer-substring-filter)
  (set (make-local-variable 'isearch-filter-predicate)          #'logview--isearch-filter-predicate)
  (add-hook 'after-change-functions #'logview--invalidate-region-entries nil t)
  (add-hook 'change-major-mode-hook #'logview--exiting-mode nil t)
  (logview--guess-submode)
  (logview--update-invisibility-spec)
  (unless (logview-initialized-p)
    (message "Cannot determine log format; press C-c C-c to choose manually or C-c C-s to customize relevant options")))

This helps a bit as if I then use this in my code:

(define-derived-mode robot-log-mode logview-mode
  "Robot-Log"
  "Major mode for viewing RobotFramework debug logs.

\\{robot-log-mode-map}"
  ;;(font-lock-add-keywords nil robot-log-font-lock-keywords t)

  (setq font-lock-defaults `(robot-log-font-lock-keywords
                             nil nil nil
                             nil;(font-lock-fontify-region-function . ,'logview--fontify-region)
                             )
        imenu-generic-expression robot-log-imenu-generic-expression))

I at least get my font lock keywords. Replacing the trailing nil with (font-lock-fontify-region-function . ,'logview--fontify-region) in font-lock-defaults, I get only logview's font lock though, so my composition problem still isn't solved.

[Apteryks]

Attached a sample debug.log file from RobotFramework to exercise the mode with: debug.log

[doublep]

I currently don't have time or desire to work on the mode heavily, but will accept (with necessary polish as I see fit) any sane PR you might produce. In short: logview--fontify-region is needed for high-performance fontifying of huge log files, see commit 664c5d4. It cannot be replaced with only standard Emacs functionality. However, it can probably be improved to honor font-lock-keywords and other standard things when those are set, likely by a submode, as in your case. I would start by looking at font-lock-default-fontify-region and copying/adapting relevant parts into logview--fontify-region.

Bare minimum for a PR: raw Logview must work as before, performance must not suffer (if every addition is conditioned on checking variables that will be normally nil anyway, performance will certainly not be affected). It should also make deriving possible, but this you must be interested in yourself.

Alternatively to creating a derived mode, you can try merging at least some of functionality you need into Logview itself. However, it needs to be generic enough, i.e. of use to at least some other people too. Before heavily investing time into this, I recommend you formulate what exactly you are going to do to see if I'm interested.

[Apteryks]

Hi, and thanks for the reply.

Bare minimum for a PR: raw Logview must work as before, performance must not suffer (if every addition is conditioned on checking variables that will be normally nil anyway, performance will certainly not be affected). It should also make deriving possible, but this you must be interested in yourself.

If I get to it, this is the route I'll pursue since it would provide the most flexibility and add the less code to logview itself.

[Apteryks]

So I tried this, thinking perhaps if I could do my font lock things first, it'd layer on top. Unfortunately, it seems to not preserve existing font lock properties. Here's the failed attempt:

1 file changed, 14 insertions(+), 1 deletion(-)
logview.el | 15 ++++++++++++++-

modified   logview.el
@@ -1051,6 +1051,14 @@ Mode is inactive when the buffer is not read-only (to not
 interfere with editing) or if submode wasn't guessed
 successfully.")

+(defvar logview-mode-font-lock-defaults
+  `(nil nil nil nil
+        (font-lock-fontify-region-function . ,'logview--fontify-region))
+  "The default FONT-LOCK-DEFAULTS value used by `logview-mode'.")
+
+(defun custom-font-lock-defaults-p ()
+    "Predicate to verify if FONT-LOCK-DEFAULTS has been overridden."
+    (not (equal logview-mode-font-lock-defaults font-lock-defaults)))

 ;;;###autoload
 (define-derived-mode logview-mode nil "Logview"
@@ -3238,7 +3246,12 @@ This list is preserved across Emacs session in
             (setq region-end (or (next-single-property-change region-end 'logview-entry) (point-max))))))
       (remove-list-of-text-properties region-start region-end '(logview-entry fontified)))))

-(defun logview--fontify-region (region-start region-end _loudly)
+(defun logview--fontify-region (region-start region-end loudly)
+  ;; After logview's fontifying routine is done,
+  ;; invoke the normal fontifying mechanism when
+  ;; FONT-LOCK-DEFAULTS appear customized.
+  (when (custom-font-lock-defaults-p)
+    (font-lock-default-fontify-region region-start region-end loudly))
   (when (logview-initialized-p)
     (logview--std-temporarily-widening
       ;; We are very fast.  Don't fontify too little to avoid overhead.

The end result is as if I hadn't changed anything. If I comment the rest of the proc (the (when ...) tree), I see my font lock keywords applied.

[doublep]

I currently don't have time or desire to work on the mode

Found an old unfinished stash together with desire to finish it. If you are still interested, I can look into this font-lock stuff for 0.15 too.

[Apteryks]

Neat! I went full custom for now, due to this and other issues I was struggling with when trying to derive my mode from logview, but perhaps I could revisit this at a later point (see: https://git.sr.ht/~apteryx/emacs-robot-log/tree/main/item/robot-log.el).

[doublep]

See file test/logview.el for an example of a derived mode with additional fontification. I will release 0.15 with this soon, likely in a few days.

[doublep]

Released Logview 0.15 with these improvements. As I haven't got any feedback, I'll assume they have fixed the problem.