Re: [ELPA] New package: ert-font-lock

[Philip Kaludercic]

Vladimir Kazanov <vekazanov@gmail.com> writes:

> Hi all,
>
> I want to propose a new package to be included in ELPA. ert-font-lock
> (ERT Font Lock) is an extension to the standard ERT unit testing tool
> that makes it possible to write font-locking tests using a
> comment-based syntax. The syntax itself is based on the Tree-sitter
> unit testing system
> (https://tree-sitter.github.io/tree-sitter/syntax-highlighting#unit-testing).
>
> Find the package along with a test suite and a README here:
> https://github.com/vkazanov/ert-font-lock

Here are a few comments from reading over the source code:

diff --git a/ert-font-lock.el b/ert-font-lock.el
index 7b8df01..6a6593f 100644
--- a/ert-font-lock.el
+++ b/ert-font-lock.el
@@ -6,7 +6,7 @@
 ;; Keywords: lisp, test
 ;; URL: https://github.com/vkazanov/ert-font-lock
 ;; Version: 0.1.0
-;; Package-Requires: ((emacs "29.1"))
+;; Package-Requires: ((emacs "28.1"))
 
 ;; This program is free software; you can redistribute it and/or modify
 ;; it under the terms of the GNU General Public License as published by
@@ -61,7 +61,7 @@
   "Validate if MODE is a valid major mode."
   (unless (functionp mode)
     (user-error "Invalid major mode: %s. Please specify a valid major mode for
-syntax highlighting tests" mode)))
+ syntax highlighting tests" mode)))
 
 
 (defmacro ert-font-lock-deftest (name mode test-string &optional docstring)
@@ -69,6 +69,7 @@ syntax highlighting tests" mode)))
 TEST-STRING is the string to test, MODE is the major mode, and
 DOCSTRING is a docstring to use for the test."
   (declare (indent 2) (debug t) (doc-string 4))
+  ;; Or would it be possible to define a function that calls `ert-set-test'?
   `(ert-deftest ,name ()
      ,@(when docstring `(,docstring))
      (ert-font-lock--validate-major-mode ',mode)
@@ -79,7 +80,6 @@ DOCSTRING is a docstring to use for the test."
        (let ((tests (ert-font-lock--parse-comments)))
          (ert-font-lock--check-faces tests)))))
 
-
 (defmacro ert-font-lock-deftest-file (name mode file &optional docstring)
   "Define an ERT test NAME for font-lock syntax highlighting.
 FILE is the path to a file in ert resource dir with test cases,
@@ -91,23 +91,24 @@ the test."
      (ert-font-lock--validate-major-mode ',mode)
      (ert-font-lock-test-file (ert-resource-file ,file) ',mode)))
 
-
 (defun ert-font-lock--line-comment-p ()
   "Return t if the current line is a comment-only line."
+  (syntax-ppss)
   (save-excursion
     (beginning-of-line)
     (skip-syntax-forward " ")
-    ;; skip empty lines
-    (unless (eolp)
-      (or
-       ;; try the most convenient approach
-       (looking-at "\\s<")
-       ;; a bit smarter
-       (and comment-start (looking-at (regexp-quote comment-start)))
-       (and comment-start-skip (looking-at comment-start-skip))
-       ;; hardcoded
-       (and (derived-mode-p 'c-mode 'c++-mode 'java-mode)
-            (looking-at-p "//"))))))
+    (or
+     ;; skip empty lines
+     (eolp)
+     ;; try the most convenient approach
+     (looking-at "\\s<")
+     ;; a bit smarter
+     (and comment-start (looking-at (regexp-quote comment-start)))
+     (and comment-start-skip (looking-at comment-start-skip))
+     ;; hardcoded
+     (cond
+      ((derived-mode-p 'c-mode 'c++-mode 'java-mode)
+       (looking-at-p "//"))))))
 
 (defun ert-font-lock--goto-first-char ()
   "Move the point to the first character."
@@ -143,7 +144,7 @@ the test."
                                  (line-end-position) t)
 
           (unless (> linetocheck -1)
-            (user-error "Invalid test comment syntax at line %d. Expected a 
line to test before the comment line" curline))
+            (user-error "Invalid test comment syntax at line %d. Expected a 
line to test before the comment line" curline)) ;is this a user error?
 
           ;; construct a test
           (let* (;; either comment start char column (for arrows) or
@@ -243,5 +244,4 @@ The function is meant to be run from within an ERT test."
 
 
 (provide 'ert-font-lock)
-
 ;;; ert-font-lock.el ends here

> I am the sole author of the package, and did sign FSF papers some time
> ago so this should not be an issue.
>
> Comments, suggestions and critique are very welcome as the package is
> very new. I am open to ideas on the best places to publish the package
> if ELPA is not suitable for it.

ELPA shouls be fine.

> Some additional context.
>
> A while ago I created quakec-mode
> (https://github.com/vkazanov/quakec-mode). One of the most painful
> things about the mode is regex-based syntax highlighting. So I turned
> to creating a Tree-sitter grammar
> (https://github.com/vkazanov/tree-sitter-quakec) as well as a TS-based
> mode (https://github.com/vkazanov/quakec-ts-mode).
>
> While doing the syntax highlighting part proved to be much, much
> easier, I couldn't do work without relying on some kind of unit tests.
> Existing font-lock systems didn't feel convenient compared to the way
> Tree-sitter specifies parser tests so I replicated that.
>
> Thank you