Code quality of some -ts-mode major modes

[Philip Kaludercic <philipk@posteo.net>]

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


Hi, I took a look at some of the new tree-sitter major modes and was
surprised at what I saw.  Without meaning to belittle anyone, there were
some basic "stylistic mistakes" that I wouldn't have expected to have
gotten merged.  I didn't look up the exact chronology, but it seems like
there has been a lot of uncritical copying between these files.

Take `yaml-ts-mode' for example.  I attach a diff with a few changes
that I would have expected to have been made before the file was merged:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff, Size: 2642 bytes --]

diff --git a/lisp/textmodes/yaml-ts-mode.el b/lisp/textmodes/yaml-ts-mode.el
index dfa8d22fb34..43e8868fc44 100644
--- a/lisp/textmodes/yaml-ts-mode.el
+++ b/lisp/textmodes/yaml-ts-mode.el
@@ -2,10 +2,10 @@
 
 ;; Copyright (C) 2022-2023 Free Software Foundation, Inc.
 
-;; Author     : Randy Taylor <dev@rjt.dev>
-;; Maintainer : Randy Taylor <dev@rjt.dev>
-;; Created    : December 2022
-;; Keywords   : yaml languages tree-sitter
+;; Author: Randy Taylor <dev@rjt.dev>
+;; Maintainer: Randy Taylor <dev@rjt.dev>
+;; Created: December 2022
+;; Keywords: languages
 
 ;; This file is part of GNU Emacs.
 
@@ -23,15 +23,18 @@
 ;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
 
 ;;; Commentary:
-;;
+
+;; This file provides basic YAML syntax highlighting using Tree
+;; Sitter.  To use the `yaml-ts-mode' major mode you will have to make
+;; sure that you have installed the appropriate grammar.
 
 ;;; Code:
 
 (require 'treesit)
 
-(declare-function treesit-parser-create "treesit.c")
+;; (declare-function treesit-parser-create "treesit.c") ;doesn't appear necessary
 
-(defvar yaml-ts-mode--syntax-table
+(defvar yaml-ts-mode-syntax-table
   (let ((table (make-syntax-table)))
     (modify-syntax-entry ?#  "<"  table)
     (modify-syntax-entry ?\n ">"  table)
@@ -59,7 +62,8 @@ yaml-ts-mode--font-lock-settings
       (null_scalar)
       (reserved_directive)
       (tag_directive)
-      (yaml_directive)] @font-lock-constant-face)
+      (yaml_directive)]
+     @font-lock-constant-face)
 
    :language 'yaml
    :feature 'delimiter
@@ -83,7 +87,8 @@ yaml-ts-mode--font-lock-settings
    '([(block_scalar)
       (double_quote_scalar)
       (single_quote_scalar)
-      (string_scalar)] @font-lock-string-face)
+      (string_scalar)]
+     @font-lock-string-face)
 
    :language 'yaml
    :feature 'escape-sequence
@@ -120,10 +125,9 @@ yaml-ts-mode--font-lock-settings
 ;;;###autoload
 (define-derived-mode yaml-ts-mode text-mode "YAML"
   "Major mode for editing YAML, powered by tree-sitter."
-  :group 'yaml
-  :syntax-table yaml-ts-mode--syntax-table
+  ;; :group 'yaml ;; no such customisation group was defined?
 
-  (when (treesit-ready-p 'yaml)
+  (when (treesit-ready-p 'yaml)         ;why not raise an `user-error'?
     (treesit-parser-create 'yaml)
 
     ;; Comments.
@@ -143,9 +147,8 @@ yaml-ts-mode
 
     (treesit-major-mode-setup)))
 
-(if (treesit-ready-p 'yaml)
-    (add-to-list 'auto-mode-alist '("\\.ya?ml\\'" . yaml-ts-mode)))
+(when (treesit-ready-p 'yaml)
+  (add-to-list 'auto-mode-alist '("\\.ya?ml\\'" . yaml-ts-mode)))
 
 (provide 'yaml-ts-mode)
-
 ;;; yaml-ts-mode.el ends here

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


In particular: The lack of a commentary section or any
indication/pointer on how to install the grammar which is the necessary
prequisite for the mode to have any effect to begin with (my
understanding is that Emacs will not ship with these files, nor are any
distributions working on providing them, right?).  I considered
mentioning the new command `treesit-install-language-grammar', but that
seems pointless as long as `treesit-language-source-alist' is empty by
default.

My question: Would there be any objection from those involves with
tree-sitter against me making changes like the ones I gave above?
Maintaining some basic style in the core seems desirable to me, as we
have seen that these files often serve as a template for creating new
major modes.