unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [ELPA] New package: ert-font-lock
@ 2023-11-18 10:43 Vladimir Kazanov
  2023-11-18 11:18 ` Philip Kaludercic
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Kazanov @ 2023-11-18 10:43 UTC (permalink / raw)
  To: emacs-devel

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

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.

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

-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 10:43 [ELPA] New package: ert-font-lock Vladimir Kazanov
@ 2023-11-18 11:18 ` Philip Kaludercic
  2023-11-18 12:07   ` Po Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Philip Kaludercic @ 2023-11-18 11:18 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

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:


[-- Attachment #2: Type: text/plain, Size: 3441 bytes --]

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

[-- Attachment #3: Type: text/plain, Size: 971 bytes --]


> 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 11:18 ` Philip Kaludercic
@ 2023-11-18 12:07   ` Po Lu
  2023-11-18 12:43     ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Po Lu @ 2023-11-18 12:07 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Vladimir Kazanov, emacs-devel

Philip Kaludercic <philipk@posteo.net> writes:

> ELPA shouls be fine.

Since this is an adjunct to ERT, which is plausibly useful for the unit
testing of Emacs fontification code itself, I think such code should be
part of Emacs proper.

BTW, this code requires cl-lib for a meager one macro.  Please replace

      (cl-incf curline)

with

      (setq curline (1+ curline))

that cl-lib might not be loaded either at compile-time or runtime.
There is no rationale for requiring cl-lib so as to employ a single
macro once.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 12:07   ` Po Lu
@ 2023-11-18 12:43     ` Eli Zaretskii
  2023-11-18 13:14       ` Po Lu
                         ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Eli Zaretskii @ 2023-11-18 12:43 UTC (permalink / raw)
  To: Po Lu; +Cc: philipk, vekazanov, emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Cc: Vladimir Kazanov <vekazanov@gmail.com>,  emacs-devel@gnu.org
> Date: Sat, 18 Nov 2023 20:07:42 +0800
> 
> Philip Kaludercic <philipk@posteo.net> writes:
> 
> > ELPA shouls be fine.
> 
> Since this is an adjunct to ERT, which is plausibly useful for the unit
> testing of Emacs fontification code itself, I think such code should be
> part of Emacs proper.

I won't object to including this, FWIW.

> BTW, this code requires cl-lib for a meager one macro.  Please replace
> 
>       (cl-incf curline)
> 
> with
> 
>       (setq curline (1+ curline))
> 
> that cl-lib might not be loaded either at compile-time or runtime.
> There is no rationale for requiring cl-lib so as to employ a single
> macro once.

Doesn't this library require ert?  if it does, cl-lib is already
loaded by ert.

But if people like cl-incf so much, we could just add incf to subr.el
or something.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 12:43     ` Eli Zaretskii
@ 2023-11-18 13:14       ` Po Lu
  2023-11-18 14:47         ` Philip Kaludercic
  2023-11-18 14:18       ` john muhl
  2023-11-19 10:08       ` Vladimir Kazanov
  2 siblings, 1 reply; 16+ messages in thread
From: Po Lu @ 2023-11-18 13:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: philipk, vekazanov, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Doesn't this library require ert?  if it does, cl-lib is already
> loaded by ert.

That's a disappointment, but let's not double the problem.

> But if people like cl-incf so much, we could just add incf to subr.el
> or something.

Fine by me, thanks.  (Don't we have incf elsewhere in core?  Or was it
in the old cl.el?)



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 12:43     ` Eli Zaretskii
  2023-11-18 13:14       ` Po Lu
@ 2023-11-18 14:18       ` john muhl
  2023-11-19 10:02         ` Vladimir Kazanov
  2023-11-19 10:08       ` Vladimir Kazanov
  2 siblings, 1 reply; 16+ messages in thread
From: john muhl @ 2023-11-18 14:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, philipk, vekazanov, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: Vladimir Kazanov <vekazanov@gmail.com>,  emacs-devel@gnu.org
>> Date: Sat, 18 Nov 2023 20:07:42 +0800
>> 
>> Philip Kaludercic <philipk@posteo.net> writes:
>> 
>> > ELPA shouls be fine.
>> 
>> Since this is an adjunct to ERT, which is plausibly useful for the unit
>> testing of Emacs fontification code itself, I think such code should be
>> part of Emacs proper.
>
> I won't object to including this, FWIW.

faceup.el is already included and seems to provide similar
functionality. Vladimir could you provide a comparison between
your package and faceup?



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 13:14       ` Po Lu
@ 2023-11-18 14:47         ` Philip Kaludercic
  2023-11-18 16:23           ` Eli Zaretskii
  2023-11-19  9:39           ` Vladimir Kazanov
  0 siblings, 2 replies; 16+ messages in thread
From: Philip Kaludercic @ 2023-11-18 14:47 UTC (permalink / raw)
  To: Po Lu; +Cc: Eli Zaretskii, vekazanov, emacs-devel

Po Lu <luangruo@yahoo.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Doesn't this library require ert?  if it does, cl-lib is already
>> loaded by ert.
>
> That's a disappointment, but let's not double the problem.

The problem, as in the usage of cl-lib?  You are probably referring to
the other thread about using CL in Elisp, right?

>> But if people like cl-incf so much, we could just add incf to subr.el
>> or something.
>
> Fine by me, thanks.  (Don't we have incf elsewhere in core?  Or was it
> in the old cl.el?)

That is part of cl, and is aliased to cl-incf.

-- 
Philip Kaludercic



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 14:47         ` Philip Kaludercic
@ 2023-11-18 16:23           ` Eli Zaretskii
  2023-11-19  9:39           ` Vladimir Kazanov
  1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2023-11-18 16:23 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: luangruo, vekazanov, emacs-devel

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  vekazanov@gmail.com,  emacs-devel@gnu.org
> Date: Sat, 18 Nov 2023 14:47:51 +0000
> 
> >> But if people like cl-incf so much, we could just add incf to subr.el
> >> or something.
> >
> > Fine by me, thanks.  (Don't we have incf elsewhere in core?  Or was it
> > in the old cl.el?)
> 
> That is part of cl, and is aliased to cl-incf.

That's not relevant.  I meant a new function, and if the name incf is
already taken, we could use a different name.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 14:47         ` Philip Kaludercic
  2023-11-18 16:23           ` Eli Zaretskii
@ 2023-11-19  9:39           ` Vladimir Kazanov
  1 sibling, 0 replies; 16+ messages in thread
From: Vladimir Kazanov @ 2023-11-19  9:39 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel

Hi Philip,

Thank you for taking a look at this!

I've integrated most of the changes but the ones below:

> +  ;; Or would it be possible to define a function that calls `ert-set-test'?

Sure, not a problem. But wouldn't the test implementation on this
level be just a replication of the already existing ert-deftest macro?
And ert-deftest, btw, does use half of cl-lib. Removing cl-incf just
stops making sense.

Admittedly, I don't have a bigger picture on Emacs Lisp idioms and
good practices so I'll just do whatever is suggested.

> +  (user-error "Invalid test comment syntax at line %d. Expected a line to test before the comment line" curline)) ;is this a user error?

Well, I am on the fence about this one. My line of thought was that
submitting invalid test cases is very much a user error, isn't it?

> +    (or
> +     ;; skip empty lines
> +     (eolp)

I like the trick. But on empty lines this would return t instead of
only reporting comment lines.

On Sat, 18 Nov 2023 at 14:47, Philip Kaludercic <philipk@posteo.net> wrote:
>
> Po Lu <luangruo@yahoo.com> writes:
>
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >> Doesn't this library require ert?  if it does, cl-lib is already
> >> loaded by ert.
> >
> > That's a disappointment, but let's not double the problem.
>
> The problem, as in the usage of cl-lib?  You are probably referring to
> the other thread about using CL in Elisp, right?
>
> >> But if people like cl-incf so much, we could just add incf to subr.el
> >> or something.
> >
> > Fine by me, thanks.  (Don't we have incf elsewhere in core?  Or was it
> > in the old cl.el?)
>
> That is part of cl, and is aliased to cl-incf.
>
> --
> Philip Kaludercic



-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 14:18       ` john muhl
@ 2023-11-19 10:02         ` Vladimir Kazanov
  0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Kazanov @ 2023-11-19 10:02 UTC (permalink / raw)
  To: jm; +Cc: Eli Zaretskii, Po Lu, philipk, emacs-devel

> faceup.el is already included and seems to provide similar
> functionality. Vladimir could you provide a comparison between
> your package and faceup?

Sure!

I use Anders' set of tools intensively but faceup.el is not exactly
what I was looking for. Let me explain.

faceup.el dumps the current state of highlighting into a separate file
and makes it possible to report any differences. This makes it
possible to "freeze" some state and then make sure nothing happens in
the bigger picture. It provides a relatively developed markup language
that surfaces most of the highlighting features of Emacs. The library
is about 700 LOC excluding the header.

So to me faceup.el is a way to save and check full snapshots of syntax
highlighting. This is an *extremely* useful tool for tweaking an
already existing complicated highlighting system.

ert-font-lock.el is the simpler of the two (maybe 200 LOC). It
introduces a way to non-intrusively say something: "check that the
face above is right", and that's about it. The other benefit is that
it's modelled on Tree-sitter unit tests which makes porting already
existing TS grammar highlighting unit tests trivial.

So instead of snapshot-oriented testing this becomes closer to the
original idea of unit tests: adding small tests incrementally.

-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-18 12:43     ` Eli Zaretskii
  2023-11-18 13:14       ` Po Lu
  2023-11-18 14:18       ` john muhl
@ 2023-11-19 10:08       ` Vladimir Kazanov
  2023-11-20 18:27         ` Vladimir Kazanov
  2 siblings, 1 reply; 16+ messages in thread
From: Vladimir Kazanov @ 2023-11-19 10:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, philipk, emacs-devel

> I won't object to including this, FWIW.

Would this require any additional changes to the way the code is structured?

There were also some questions around ert-font-lock.el being similar
to faceup.el. I covered these elsewhere.

On Sat, 18 Nov 2023 at 12:43, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Po Lu <luangruo@yahoo.com>
> > Cc: Vladimir Kazanov <vekazanov@gmail.com>,  emacs-devel@gnu.org
> > Date: Sat, 18 Nov 2023 20:07:42 +0800
> >
> > Philip Kaludercic <philipk@posteo.net> writes:
> >
> > > ELPA shouls be fine.
> >
> > Since this is an adjunct to ERT, which is plausibly useful for the unit
> > testing of Emacs fontification code itself, I think such code should be
> > part of Emacs proper.
>
> I won't object to including this, FWIW.
>
> > BTW, this code requires cl-lib for a meager one macro.  Please replace
> >
> >       (cl-incf curline)
> >
> > with
> >
> >       (setq curline (1+ curline))
> >
> > that cl-lib might not be loaded either at compile-time or runtime.
> > There is no rationale for requiring cl-lib so as to employ a single
> > macro once.
>
> Doesn't this library require ert?  if it does, cl-lib is already
> loaded by ert.
>
> But if people like cl-incf so much, we could just add incf to subr.el
> or something.



-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-19 10:08       ` Vladimir Kazanov
@ 2023-11-20 18:27         ` Vladimir Kazanov
  2023-11-21  4:45           ` john muhl
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Kazanov @ 2023-11-20 18:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, philipk, emacs-devel

Hi everyone,

Just bumping this thread for visibility.

I've made most of the changes that were suggested earlier and
clarified how my package differs from faceup.el. I'm also considering
if it's feasible to lower the Emacs version requirement to 27.1 and
planning to run some more tests for edge cases. Other than that, the
tool is pretty much set and shouldn't see major changes.

Could you let me know what I should do next to move forward with
adding the package to ELPA (or Emacs itself if Eli doesn't mind)?

Thanks a lot for your help!


On Sun, 19 Nov 2023 at 10:08, Vladimir Kazanov <vekazanov@gmail.com> wrote:
>
> > I won't object to including this, FWIW.
>
> Would this require any additional changes to the way the code is structured?
>
> There were also some questions around ert-font-lock.el being similar
> to faceup.el. I covered these elsewhere.
>
> On Sat, 18 Nov 2023 at 12:43, Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > From: Po Lu <luangruo@yahoo.com>
> > > Cc: Vladimir Kazanov <vekazanov@gmail.com>,  emacs-devel@gnu.org
> > > Date: Sat, 18 Nov 2023 20:07:42 +0800
> > >
> > > Philip Kaludercic <philipk@posteo.net> writes:
> > >
> > > > ELPA shouls be fine.
> > >
> > > Since this is an adjunct to ERT, which is plausibly useful for the unit
> > > testing of Emacs fontification code itself, I think such code should be
> > > part of Emacs proper.
> >
> > I won't object to including this, FWIW.
> >
> > > BTW, this code requires cl-lib for a meager one macro.  Please replace
> > >
> > >       (cl-incf curline)
> > >
> > > with
> > >
> > >       (setq curline (1+ curline))
> > >
> > > that cl-lib might not be loaded either at compile-time or runtime.
> > > There is no rationale for requiring cl-lib so as to employ a single
> > > macro once.
> >
> > Doesn't this library require ert?  if it does, cl-lib is already
> > loaded by ert.
> >
> > But if people like cl-incf so much, we could just add incf to subr.el
> > or something.
>
>
>
> --
> Regards,
>
> Vladimir Kazanov



--
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-20 18:27         ` Vladimir Kazanov
@ 2023-11-21  4:45           ` john muhl
  2023-11-21  7:43             ` Vladimir Kazanov
  0 siblings, 1 reply; 16+ messages in thread
From: john muhl @ 2023-11-21  4:45 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: Eli Zaretskii, Po Lu, philipk, emacs-devel

Vladimir Kazanov <vekazanov@gmail.com> writes:

> Hi everyone,
>
> Just bumping this thread for visibility.
>
> I've made most of the changes that were suggested earlier and
> clarified how my package differs from faceup.el. I'm also considering
> if it's feasible to lower the Emacs version requirement to 27.1 and
> planning to run some more tests for edge cases. Other than that, the
> tool is pretty much set and shouldn't see major changes.
>
> Could you let me know what I should do next to move forward with
> adding the package to ELPA (or Emacs itself if Eli doesn't mind)?
>
> Thanks a lot for your help!

FWIW I used it to add tests for lua-ts-mode’s font-lock rules.
It works well and feels to me like it would be a nice addition
to Emacs. Thanks for working on it.

> On Sun, 19 Nov 2023 at 10:08, Vladimir Kazanov <vekazanov@gmail.com> wrote:
>>
>> > I won't object to including this, FWIW.
>>
>> Would this require any additional changes to the way the code is structured?
>>
>> There were also some questions around ert-font-lock.el being similar
>> to faceup.el. I covered these elsewhere.
>>
>> On Sat, 18 Nov 2023 at 12:43, Eli Zaretskii <eliz@gnu.org> wrote:
>> >
>> > > From: Po Lu <luangruo@yahoo.com>
>> > > Cc: Vladimir Kazanov <vekazanov@gmail.com>,  emacs-devel@gnu.org
>> > > Date: Sat, 18 Nov 2023 20:07:42 +0800
>> > >
>> > > Philip Kaludercic <philipk@posteo.net> writes:
>> > >
>> > > > ELPA shouls be fine.
>> > >
>> > > Since this is an adjunct to ERT, which is plausibly useful for the unit
>> > > testing of Emacs fontification code itself, I think such code should be
>> > > part of Emacs proper.
>> >
>> > I won't object to including this, FWIW.
>> >
>> > > BTW, this code requires cl-lib for a meager one macro.  Please replace
>> > >
>> > >       (cl-incf curline)
>> > >
>> > > with
>> > >
>> > >       (setq curline (1+ curline))
>> > >
>> > > that cl-lib might not be loaded either at compile-time or runtime.
>> > > There is no rationale for requiring cl-lib so as to employ a single
>> > > macro once.
>> >
>> > Doesn't this library require ert?  if it does, cl-lib is already
>> > loaded by ert.
>> >
>> > But if people like cl-incf so much, we could just add incf to subr.el
>> > or something.
>>
>>
>>
>> --
>> Regards,
>>
>> Vladimir Kazanov




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-21  4:45           ` john muhl
@ 2023-11-21  7:43             ` Vladimir Kazanov
  2023-11-21 16:51               ` john muhl
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Kazanov @ 2023-11-21  7:43 UTC (permalink / raw)
  To: john muhl; +Cc: Eli Zaretskii, Po Lu, philipk, emacs-devel

On Tue, 21 Nov 2023 at 04:48, john muhl <jm@pub.pink> wrote:

> FWIW I used it to add tests for lua-ts-mode’s font-lock rules.
> It works well and feels to me like it would be a nice addition
> to Emacs. Thanks for working on it.

Hey, that's nice to hear!

Can I take a look at the code? Curious about the way comment
highlighting is implemented in your mode.

I did test popular Emacs major modes (cc-mode and the like, python,
php, etc) but few ts-based ones. And even in the pre-Tree-sitter world
comment detection is rather unstable and needs custom hacks in some
cases.

Thanks,
Vlad

--

Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-21  7:43             ` Vladimir Kazanov
@ 2023-11-21 16:51               ` john muhl
  2023-11-23 13:00                 ` Vladimir Kazanov
  0 siblings, 1 reply; 16+ messages in thread
From: john muhl @ 2023-11-21 16:51 UTC (permalink / raw)
  To: Vladimir Kazanov; +Cc: Eli Zaretskii, Po Lu, philipk, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

Vladimir Kazanov <vekazanov@gmail.com> writes:

> On Tue, 21 Nov 2023 at 04:48, john muhl <jm@pub.pink> wrote:
>
>> FWIW I used it to add tests for lua-ts-mode’s font-lock rules.
>> It works well and feels to me like it would be a nice addition
>> to Emacs. Thanks for working on it.
>
> Hey, that's nice to hear!
>
> Can I take a look at the code? Curious about the way comment
> highlighting is implemented in your mode.

https://git.sv.gnu.org/cgit/emacs.git/tree/lisp/progmodes/lua-ts-mode.el

Attached are the tests. They fail against master since some bugs
were found in the process.

> I did test popular Emacs major modes (cc-mode and the like, python,
> php, etc) but few ts-based ones. And even in the pre-Tree-sitter world
> comment detection is rather unstable and needs custom hacks in some
> cases.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-font-lock-tests-for-lua-ts-mode-ert-font-lock.patch --]
[-- Type: text/x-patch, Size: 9397 bytes --]

From 5e239acb7217192e74577a123ed172992402b318 Mon Sep 17 00:00:00 2001
From: john muhl <jm@pub.pink>
Date: Mon, 20 Nov 2023 23:02:12 -0600
Subject: [PATCH] Add font-lock tests for lua-ts-mode (ert-font-lock)

* test/lisp/progmodes/lua-ts-mode-tests.el
(lua-ts-test-font-lock): Add test.
* test/lisp/progmodes/lua-ts-mode-resources/font-lock.lua: New
file.
---
 .../lua-ts-mode-resources/font-lock.lua       | 332 ++++++++++++++++++
 test/lisp/progmodes/lua-ts-mode-tests.el      |   6 +
 2 files changed, 338 insertions(+)
 create mode 100644 test/lisp/progmodes/lua-ts-mode-resources/font-lock.lua

diff --git a/test/lisp/progmodes/lua-ts-mode-resources/font-lock.lua b/test/lisp/progmodes/lua-ts-mode-resources/font-lock.lua
new file mode 100644
index 00000000000..86bdfa2c8f1
--- /dev/null
+++ b/test/lisp/progmodes/lua-ts-mode-resources/font-lock.lua
@@ -0,0 +1,332 @@
+#!/usr/bin/env lua
+-- ^ font-lock-comment-face
+-- Comment
+-- ^ font-lock-comment-face
+--[[
+Multi-line comment
+-- ^ font-lock-comment-face
+]]
+
+-- Definition
+local function f1() end
+--             ^ font-lock-function-name-face
+local f2 = function() end
+--    ^ font-lock-function-name-face
+local tb = { f1 = function() end }
+--           ^ font-lock-function-name-face
+function tb.f2() end
+--          ^ font-lock-function-name-face
+function tb:f3() end
+--          ^ font-lock-function-name-face
+tbl.f4 = function() end
+--  ^ font-lock-function-name-face
+function x.y:z() end
+--           ^ font-lock-function-name-face
+
+-- Keyword
+if true then
+-- <- font-lock-keyword-face
+elseif true then
+-- <- font-lock-keyword-face
+else end
+-- <- font-lock-keyword-face
+--   ^ font-lock-keyword-face
+local p = {}
+-- ^ font-lock-keyword-face
+for k,v in pairs({}) do end
+-- <- font-lock-keyword-face
+--      ^ font-lock-keyword-face
+repeat if true then break end until false
+-- <- font-lock-keyword-face
+--                  ^ font-lock-keyword-face
+--                            ^ font-lock-keyword-face
+while true do end
+-- <- font-lock-keyword-face
+--         ^ font-lock-keyword-face
+function fn() return true end
+-- <- font-lock-keyword-face
+--            ^ font-lock-keyword-face
+goto label
+-- ^ font-lock-keyword-face
+::label1::
+
+-- String
+local _
+_ = "x"
+--   ^ font-lock-string-face
+_ = 'x'
+--   ^ font-lock-string-face
+_ = "x\ty"
+--   ^ font-lock-string-face
+--      ^ font-lock-string-face
+_ = "x\"y"
+--   ^ font-lock-string-face
+--      ^ font-lock-string-face
+_ = 'x\'y'
+--   ^ font-lock-string-face
+--      ^ font-lock-string-face
+_ = "x\z
+    y"
+--  ^ font-lock-string-face
+_ = "x\0900y"
+--        ^ font-lock-string-face
+_ = "x\09y"
+--       ^ font-lock-string-face
+_ = "x\0y"
+--      ^ font-lock-string-face
+_ = "x\u{1f602}y"
+--             ^ font-lock-string-face
+_ = [[x]]
+--    ^ font-lock-string-face
+_ = [=[x]=]
+--     ^ font-lock-string-face
+
+-- Assignment
+local n = 0
+--    ^ font-lock-variable-name-face
+o, p, q = 1, 2, 3
+-- <- font-lock-variable-name-face
+-- ^ font-lock-variable-name-face
+--    ^ font-lock-variable-name-face
+tbl[k] = "A"
+--  ^ font-lock-variable-name-face
+tbl.x = 1
+--  ^ font-lock-variable-name-face
+for i=0,9 do end
+--  ^ font-lock-variable-name-face
+
+-- Constant
+local x <const> = 1
+--      ^ font-lock-constant-face
+local f <close> = io.open('/file')
+--      ^ font-lock-constant-face
+local a, b, c = true, false, nil
+--              ^ font-lock-constant-face
+--                    ^ font-lock-constant-face
+--                           ^ font-lock-constant-face
+::label2::
+-- ^ font-lock-constant-face
+
+-- Number
+n = 123
+--  ^ font-lock-number-face
+print(99)
+--    ^ font-lock-number-face
+tbl[1]
+--  ^ font-lock-number-face
+
+-- Bracket
+local t = {}
+--        ^ font-lock-bracket-face
+--         ^ font-lock-bracket-face
+print(t[1])
+--   ^ font-lock-bracket-face
+--     ^ font-lock-bracket-face
+--       ^ font-lock-bracket-face
+--        ^ font-lock-bracket-face
+
+-- Builtin
+assert()
+-- <- font-lock-builtin-face
+bit32()
+-- <- font-lock-builtin-face
+collectgarbage()
+-- <- font-lock-builtin-face
+coroutine()
+-- <- font-lock-builtin-face
+debug()
+-- <- font-lock-builtin-face
+dofile()
+-- <- font-lock-builtin-face
+error()
+-- <- font-lock-builtin-face
+getmetatable()
+-- <- font-lock-builtin-face
+io()
+-- <- font-lock-builtin-face
+ipairs()
+-- <- font-lock-builtin-face
+load()
+-- <- font-lock-builtin-face
+loadfile()
+-- <- font-lock-builtin-face
+math()
+-- <- font-lock-builtin-face
+next()
+-- <- font-lock-builtin-face
+os()
+-- <- font-lock-builtin-face
+package()
+-- <- font-lock-builtin-face
+pairs()
+-- <- font-lock-builtin-face
+pcall()
+-- <- font-lock-builtin-face
+print()
+-- <- font-lock-builtin-face
+rawequal()
+-- <- font-lock-builtin-face
+rawget()
+-- <- font-lock-builtin-face
+rawlen()
+-- <- font-lock-builtin-face
+rawset()
+-- <- font-lock-builtin-face
+require()
+-- <- font-lock-builtin-face
+select()
+-- <- font-lock-builtin-face
+setmetatable()
+-- <- font-lock-builtin-face
+string()
+-- <- font-lock-builtin-face
+table()
+-- <- font-lock-builtin-face
+tonumber()
+-- <- font-lock-builtin-face
+tostring()
+-- <- font-lock-builtin-face
+type()
+-- <- font-lock-builtin-face
+utf8()
+-- <- font-lock-builtin-face
+warn()
+-- <- font-lock-builtin-face
+xpcall()
+-- <- font-lock-builtin-face
+print(_G)
+--    ^ font-lock-builtin-face
+print(_VERSION)
+--    ^ font-lock-builtin-face
+f.close()
+-- ^ font-lock-builtin-face
+f.flush()
+-- ^ font-lock-builtin-face
+f.lines()
+-- ^ font-lock-builtin-face
+f.read()
+-- ^ font-lock-builtin-face
+f.seek()
+-- ^ font-lock-builtin-face
+f.setvbuf()
+-- ^ font-lock-builtin-face
+f.write()
+-- ^ font-lock-builtin-face
+
+-- Delimiter
+t = { 1, 2 };
+--     ^ font-lock-delimiter-face
+--          ^ font-lock-delimiter-face
+
+-- Escape
+_ = "x\ty"
+--    ^ font-lock-escape-face
+--     ^ font-lock-escape-face
+_ = "x\"y"
+--    ^ font-lock-escape-face
+--     ^ font-lock-escape-face
+_ = 'x\'y'
+--    ^ font-lock-escape-face
+--     ^ font-lock-escape-face
+_ = "x\z
+    y"
+-- <- font-lock-escape-face
+_ = "x\x5Ay"
+--     ^ font-lock-escape-face
+--      ^ font-lock-escape-face
+_ = "x\0900y"
+--       ^ font-lock-escape-face
+_ = "x\09y"
+--      ^ font-lock-escape-face
+_ = "x\0y"
+--     ^ font-lock-escape-face
+_ = "x\u{1f602}y"
+--     ^ font-lock-escape-face
+--         ^ font-lock-escape-face
+
+-- Function
+func_one()
+--  ^ font-lock-function-call-face
+tbl.func_two()
+--  ^ font-lock-function-call-face
+tbl:func_three()
+--  ^ font-lock-function-call-face
+tbl.f = f4()
+--      ^ font-lock-function-call-face
+
+-- Operator
+local a, b = 1, 2
+--         ^ font-lock-operator-face
+print(a & b)
+--      ^ font-lock-operator-face
+print(a | b)
+--      ^ font-lock-operator-face
+print(a ~ b)
+--      ^ font-lock-operator-face
+print(a << 1)
+--      ^ font-lock-operator-face
+--       ^ font-lock-operator-face
+print(a >> 1)
+--      ^ font-lock-operator-face
+--       ^ font-lock-operator-face
+print(a and b)
+--      ^ font-lock-operator-face
+print(a or b)
+--      ^ font-lock-operator-face
+print(not a)
+--      ^ font-lock-operator-face
+print(a+b-a*b/a%b^a//b)
+--     ^ font-lock-operator-face
+--       ^ font-lock-operator-face
+--         ^ font-lock-operator-face
+--           ^ font-lock-operator-face
+--             ^ font-lock-operator-face
+--               ^ font-lock-operator-face
+--                 ^ font-lock-operator-face
+print(#t)
+--    ^ font-lock-operator-face
+print("h".."at")
+--       ^ font-lock-operator-face
+print(a==b)
+--     ^ font-lock-operator-face
+print(a~=b)
+--     ^ font-lock-operator-face
+print(a<=b)
+--     ^ font-lock-operator-face
+print(a>=b)
+--     ^ font-lock-operator-face
+print(a<b)
+--     ^ font-lock-operator-face
+print(a>b)
+--     ^ font-lock-operator-face
+function ff(...) end
+--          ^ font-lock-operator-face
+
+-- Property
+t = { a=1 }
+--    ^ font-lock-property-name-face
+print(t.a)
+--      ^ font-lock-property-use-face
+
+-- Punctuation
+tbl.f2()
+-- ^ font-lock-punctuation-face
+tbl:f3()
+-- ^ font-lock-punctuation-face
+
+-- Variable
+function fn(x, y) end
+--          ^ font-lock-variable-name-face
+--             ^ font-lock-variable-name-face
+fn(a, b)
+-- ^ font-lock-variable-use-face
+--    ^ font-lock-variable-use-face
+print(a + b)
+--    ^ font-lock-variable-use-face
+--        ^ font-lock-variable-use-face
+print(t[a])
+--      ^ font-lock-variable-use-face
+tbl.f1(p)
+--     ^ font-lock-variable-use-face
+tbl:f2(q)
+--     ^ font-lock-variable-use-face
diff --git a/test/lisp/progmodes/lua-ts-mode-tests.el b/test/lisp/progmodes/lua-ts-mode-tests.el
index d2105b66f6d..c644280f90d 100644
--- a/test/lisp/progmodes/lua-ts-mode-tests.el
+++ b/test/lisp/progmodes/lua-ts-mode-tests.el
@@ -20,6 +20,7 @@
 ;;; Code:
 
 (require 'ert)
+(require 'ert-font-lock)
 (require 'ert-x)
 (require 'treesit)
 
@@ -31,6 +32,11 @@ lua-ts-mode-test-movement
   (skip-unless (treesit-ready-p 'lua))
   (ert-test-erts-file (ert-resource-file "movement.erts")))
 
+(ert-deftest lua-ts-test-font-lock ()
+  (skip-unless (treesit-ready-p 'lua))
+  (let ((treesit-font-lock-level 4))
+    (ert-font-lock-test-file (ert-resource-file "font-lock.lua") 'lua-ts-mode)))
+
 (provide 'lua-ts-mode-tests)
 
 ;;; lua-ts-mode-tests.el ends here
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [ELPA] New package: ert-font-lock
  2023-11-21 16:51               ` john muhl
@ 2023-11-23 13:00                 ` Vladimir Kazanov
  0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Kazanov @ 2023-11-23 13:00 UTC (permalink / raw)
  To: john muhl; +Cc: emacs-devel

On Tue, 21 Nov 2023 at 17:05, john muhl <jm@pub.pink> wrote:

>
> https://git.sv.gnu.org/cgit/emacs.git/tree/lisp/progmodes/lua-ts-mode.el
>
> Attached are the tests. They fail against master since some bugs
> were found in the process.
>

Got it, thanks!

Your code uses newcomment.el facilities so ert-font-lock.el's comment
detection code should work just fine. Based on your example I've just
added a way to check comment faces as well, i.e. something like the
following should work now :

// comment
// ^ font-lock-comment-face

or
// comment
    // <- font-lock-comment-face

This means that single line comments having no (leading "^", or "<-")
should be properly testable now as well. Previously this wouldn't work
as the code assumed all comments to be either assertions, or
non-meaningful lines.

Got to look into the multiline comment case now - you have one early
in the patch.

-- 
Regards,

Vladimir Kazanov



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-11-23 13:00 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-18 10:43 [ELPA] New package: ert-font-lock Vladimir Kazanov
2023-11-18 11:18 ` Philip Kaludercic
2023-11-18 12:07   ` Po Lu
2023-11-18 12:43     ` Eli Zaretskii
2023-11-18 13:14       ` Po Lu
2023-11-18 14:47         ` Philip Kaludercic
2023-11-18 16:23           ` Eli Zaretskii
2023-11-19  9:39           ` Vladimir Kazanov
2023-11-18 14:18       ` john muhl
2023-11-19 10:02         ` Vladimir Kazanov
2023-11-19 10:08       ` Vladimir Kazanov
2023-11-20 18:27         ` Vladimir Kazanov
2023-11-21  4:45           ` john muhl
2023-11-21  7:43             ` Vladimir Kazanov
2023-11-21 16:51               ` john muhl
2023-11-23 13:00                 ` Vladimir Kazanov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).