[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Jostein Kjønigsen
Subject: Re: Call for volunteers: add tree-sitter support to major modes
Date: Tue, 11 Oct 2022 08:53:09 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1

Testing js-mode with tree-sitter enabled, it seems more responsive than the old elisp-implementation, and it also picks up more syntax elements, like function-definitions/calls which it didn't use to.

This puts it on par with js2-mode, but probably more correct and with better performance.

All in all I would consider this a upgrade all users should want.


On 10.10.2022 09:08, Theodor Thornhill wrote:

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

In template strings in _javascript_ we have issues with precedence in that
we default to string font locking for everything that does _not_ have
specific rules.  An image speaks a thousand words, so I'll add
screenshots.  What I need is some sort of way to ensure that inside of
template strings font-locking should _not_ happen

+(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.

I added some variartion of this.  I think also message is better,
because user-error makes the logic a little harder.  What do you think?

Thanks, see attached patch:

reply via email to

[Prev in Thread] Current Thread [Next in Thread]