bug#61996: 30.0.50; Submitting elixir-ts-mode and heex-ts-mode

[Eli Zaretskii]

> From: Wilhelm Kirschbaum <wkirschbaum@gmail.com>
> Cc: 61996@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no
> Date: Mon, 06 Mar 2023 21:24:11 +0200
> 
> For this release it will be good just to get the basics to work as
> is, but good to know it is an option.
> 
> Attached are the updated patches.

Thanks, a few minor comments below.

> >From 88c941067da0e34e1e9ababeb813ba51378ae2cc Mon Sep 17 00:00:00 2001
> From: Wilhelm H Kirschbaum <wkirschbaum@gmail.com>
> Date: Mon, 6 Mar 2023 21:18:04 +0200
> Subject: [PATCH 1/2] Add heex-ts-mode
> 
> ---
>  lisp/progmodes/heex-ts-mode.el                | 185 ++++++++++++++++++
>  .../heex-ts-mode-resources/indent.erts        |  47 +++++
>  test/lisp/progmodes/heex-ts-mode-tests.el     |   9 +
>  3 files changed, 241 insertions(+)
>  create mode 100644 lisp/progmodes/heex-ts-mode.el
>  create mode 100644 test/lisp/progmodes/heex-ts-mode-resources/indent.erts
>  create mode 100644 test/lisp/progmodes/heex-ts-mode-tests.el

Please accompany the changes with a commit log message according to
our conventions (see CONTRIBUTE for the conventions; search for
"ChangeLog" there).  In this case, just "New file" log should be
sufficient for the new files you add.

> +(declare-function treesit-parser-create "treesit.c")
> +(declare-function treesit-node-child "treesit.c")
> +(declare-function treesit-node-type "treesit.c")
> +(declare-function treesit-install-language-grammar "treesit.el")

AFAICS, the code uses more functions from treesit.c; please add
declare-function forms for all of them , to avoid compilation warnings
n systems where Emacs was built without tree-sitter.

> +(defun heex-ts-mode--forward-sexp (&optional arg)
> +  (interactive "^p")

Why is a command an internal function?  That is unusual, as commands
are by definition public.  It looks like you thought the double-hyphen
"--" notation is a simple delimiter between the package-name part of
the symbol name and the rest?  If so, you were mistaken: the
double-hyphen means this is an internal function/variable.  Please
review all your symbol names in this patch and rename as appropriate.

Btw, there's no need to have the prefix be the full name of the
package, as in "elixir-ts-mode-".  You could use "elixir-ts-" instead.

> +;;;###autoload
> +(define-derived-mode heex-ts-mode html-mode "Heex"

html-mode? not html-ts-mode?

> >From d13c34ed951e3e6fa473cd1bc2e955e20455022b Mon Sep 17 00:00:00 2001
> From: Wilhelm H Kirschbaum <wkirschbaum@gmail.com>
> Date: Mon, 6 Mar 2023 21:18:35 +0200
> 
> ---
>  lisp/progmodes/elixir-ts-mode.el              | 626 ++++++++++++++++++
>  .../elixir-ts-mode-resources/indent.erts      | 147 ++++
>  test/lisp/progmodes/elixir-ts-mode-tests.el   |  31 +
>  3 files changed, 804 insertions(+)
>  create mode 100644 lisp/progmodes/elixir-ts-mode.el
>  create mode 100644 test/lisp/progmodes/elixir-ts-mode-resources/indent.erts
>  create mode 100644 test/lisp/progmodes/elixir-ts-mode-tests.el

Likewise here: please add a commit log message describing the changes.

> +(declare-function treesit-parser-create "treesit.c")
> +(declare-function treesit-node-child "treesit.c")
> +(declare-function treesit-node-type "treesit.c")
> +(declare-function treesit-node-child-by-field-name "treesit.c")
> +(declare-function treesit-parser-language "treesit.c")
> +(declare-function treesit-parser-included-ranges "treesit.c")
> +(declare-function treesit-parser-list "treesit.c")
> +(declare-function treesit-node-parent "treesit.c")
> +(declare-function treesit-node-start "treesit.c")
> +(declare-function treesit-query-compile "treesit.c")
> +(declare-function treesit-install-language-grammar "treesit.el")

Please verify that you have declare-function for all the functions
from treesit.c this package uses, and only for those.

> +(defgroup elixir-ts nil
> +  "Major mode for editing Ruby code."
                             ^^^^
"Ruby"?

> +;; used to distinguish from comment-face in query match

Comments should be complete sentences: start with a capital letter and
end with a period (here and elsewhere in the patches).

> +(defface elixir-ts-font-comment-doc-identifier-face
> +  '((t (:inherit font-lock-doc-face)))
> +  "For use with @comment.doc tag.")

This doc string is too terse.  Imagine someone looking at it in a long
list of symbols, not necessarily all of them faces.  So something
like this is better:

  Face used for @comment.doc tags in Elixir files.

Likewise for other faces in the patch.

> +    (modify-syntax-entry ?@ "'" table)
> +    table)
> +  "Syntax table for `elixir-ts-mode.")
                                      ^
The closing ' quote is missing there.