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

[Yuan Fu]

> On Oct 10, 2022, at 12:08 AM, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> Hi!
> 
>> 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?
>> 
> 
> 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?

Yeah user-error is mostly the same as a message, since nobody (I think) turns 
on beeping anymore. Though I hope there is a way to warn users that’s not 
missable. Messages (and user-error) could be covered by later messages. Hmm, 
maybe we can use warnings.

For template string please see that patch that applies on top of your patch. It 
seems to work for me. (Also see screenshot attached, brackets have different 
colors because I forgot to turn off rainbow-delimiter-mode when taking the 
shot). Also some of the lines are longer than 70 columns. Could you wrap those 
lines?

Thanks!
Yuan

template-string.patch
Description: Binary data