Re: Call for volunteers: add tree-sitter support to major modes

[Yuan Fu <casouri@gmail.com>]

> On Oct 9, 2022, at 2:21 PM, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> Theodor Thornhill <theo@thornhill.no> writes:
> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>>>> Date: Sun, 09 Oct 2022 17:18:52 +0200
>>>> From: Theodor Thornhill <theo@thornhill.no>
>>>> CC: emacs-devel@gnu.org
>>>> 
>>>> BTW, I can do json as well, should be quick enough :) I'll add it in a different patch
>>> 
>>> TIA
>> 
>> So I added tree sitter support for js-json-mode, js-jsx-mode as well.
>> The changes were simple enough not to warrant a separate patch, IMO, so
>> I just updated the old one.
>> 
>> What do you think, Yuan and Eli?
> 
> A small followup patch - this should be the last to consider for now -
> sorry the many patches :)
> 
> Theodor
> <0001-Add-tree-sitter-functionality-to-js-mode.patch>

Looks good! Here are some comments.

+
+     ;; FIXME: We need to be able to set the priority for font-locking
+     ;; somehow.  We cannot just override all of the template string,
+     ;; as that would mess up interpolated expressions
+     ;;
+     ;; (template_string) @font-lock-string-face
+     (template_substitution ["${" "}"] @font-lock-constant-face)
+     )))

What exactly do you mean by priority here? Why doesn't :override t
work?

+
+(defvar js-treesit--defun-query
+  "[(class_declaration)
+    (method_definition)
+    (function_declaration)
+    (variable_declarator)] @defun")

This should be compiled.

+
+(defun js--treesit-enable ()
+  (unless (and (treesit-can-enable-p)
+               (treesit-language-available-p 'javascript))
+    (error "Tree sitter isn't available"))

I don't think we should error here, I'd displaying a message instead.

+
+  ;; Comments
+  (setq-local comment-start "// ")
+  (setq-local comment-start-skip "\\(?://+\\|/\\*+\\)\\s *")
+  (setq-local comment-end "")

I think it's best to not repeat code, could you move this outside the
(if tree-sitter) form and have it run regardless?

+(defun js--json-treesit-enable ()
+  (unless (and (treesit-can-enable-p)
+               (treesit-language-available-p 'json))
+    (error "Tree sitter isn't available"))

Same as above, IMO message is better.

Thanks,
Yuan