Re: [NonGNU ELPA] New package: blueprint-ts-mode

[Philip Kaludercic <philipk@posteo.net>]

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

Huan Thieu Nguyen <nguyenthieuhuan@gmail.com> writes:

> Hi hi,
>
> I made a tree-sitter based mode for blueprint files
> <https://github.com/huanie/blueprint-ts-mode>. I'd like to add it to
> NonGNU ELPA.
>
> With best regards!
>
> Nguyen Thieu Huan

A few suggestions and comments from my side:


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

diff --git a/blueprint-ts-mode.el b/blueprint-ts-mode.el
index 9a88399..d33e6d0 100644
--- a/blueprint-ts-mode.el
+++ b/blueprint-ts-mode.el
@@ -25,26 +25,34 @@
 ;;; Commentary:
 
 ;; Treesitter based major mode for editing Blueprint files.
-;; Blueprint is a new markup language for GTK4 user interfaces.  For more information see <https://jwestman.pages.gitlab.gnome.org/blueprint-compiler/>.
-;; This mode provides syntax highlighting, eglot integration and Treesitter based navigation.
+
+;; Blueprint is a new markup language for GTK4 user interfaces.  For
+;; more information see
+;; <https://jwestman.pages.gitlab.gnome.org/blueprint-compiler/>.
+;; This mode provides syntax highlighting, eglot integration and
+;; Treesitter based navigation.
 
 ;;; Code:
 
 (require 'treesit)
-(require 'rx)
+(eval-when-compile (require 'rx))
 (require 'eglot)
 
+(defgroup blueprint ()
+  "Tree-sitter support for Blueprint files."
+  :prefix "blueprint-ts-"
+  :group 'languages)
+
 (defcustom blueprint-ts-mode-indent-offset 2
   "Number of spaces for each indentation step in `json-ts-mode'."
-  :version "29.1"
   :type 'integer
-  :safe 'integerp
-  :group 'blueprint)
+  :safe #'integerp)
 
 (defvar blueprint-ts-mode--keywords
   '("menu" "item" "section" "styles" "using" "bind" "template")
   "Blueprint keywords for tree-sitter font-locking.")
 
+;; Why a macro?
 (defmacro blueprint-ts-mode--treesit-font-lock-rules (language &rest rules)
   "Wrapper around `treesit-font-lock-rules'.
 `LANGUAGE' is the treesitter language to use.
@@ -62,7 +70,7 @@
      ((node-is ")") parent-bol 0)
      ((node-is "]") parent-bol 0)
      ((n-p-gp "object" "child" "object_content")
-      prev-sibling 0) ;; [child_type] indent
+      prev-sibling 0) ;; [child_type] indent (is this a meaningful comment?)
      ((parent-is "object_content") parent-bol blueprint-ts-mode-indent-offset)
      ((parent-is "template") parent-bol blueprint-ts-mode-indent-offset)
      ((parent-is "styles") parent-bol blueprint-ts-mode-indent-offset)
@@ -101,7 +109,6 @@
 ;;;###autoload
 (define-derived-mode blueprint-ts-mode prog-mode "Blueprint"
   "Blueprint major mode using treesitter."
-  :group 'blueprint
   (when (treesit-ready-p 'blueprint)
     (treesit-parser-create 'blueprint)
     ;; Comments
@@ -118,7 +125,7 @@
 		(rx (or "template" "object" "menu")))
     (setq-local treesit-sentence-type-regexp (rx (or "menu_attribute" "property")))
     ;; Font-lock
-    (setq-local treesit-font-lock-level 4)
+    (setq-local treesit-font-lock-level 4) ;isn't this a user option?
     (setq-local treesit-font-lock-feature-list
 		'((comment variable)
 		  (string keyword type)
@@ -128,9 +135,8 @@
     (treesit-major-mode-setup)))
 
 (add-to-list 'auto-mode-alist '("\\.blp\\'" . blueprint-ts-mode))
-(add-to-list 'eglot-server-programs
+(add-to-list 'eglot-server-programs	;is there some way around having to load Eglot?
              '(blueprint-ts-mode . ("blueprint-compiler" "lsp")))
 
-
 (provide 'blueprint-ts-mode)
 ;;; blueprint-ts-mode.el ends here