From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jostein_Kj=c3=b8nigsen?= Newsgroups: gmane.emacs.devel Subject: Re: Call for volunteers: add tree-sitter support to major modes Date: Tue, 11 Oct 2022 08:53:09 +0200 Message-ID: References: <83czb1jrm3.fsf@gnu.org> <84436B1D-3359-487F-B997-A7F56FDEA636@thornhill.no> <83bkqljih5.fsf@gnu.org> <87fsfx6ufb.fsf@thornhill.no> <8335bxjdpf.fsf@gnu.org> <1D07A0F1-A269-410A-9D3C-8A53113FFE2F@thornhill.no> <83sfjxhuvc.fsf@gnu.org> <87bkqk7qb0.fsf@thornhill.no> <8735bw7kwm.fsf@thornhill.no> <158F25C9-A539-4515-8D49-50A7E6252960@gmail.com> <87o7uk5f5i.fsf@thornhill.no> Reply-To: jostein@kjonigsen.net Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="------------Wk0HxuiIoSON3WtwBOIEUiZn" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="3015"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Oct 11 09:16:27 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oi9V2-0000VQ-Jw for ged-emacs-devel@m.gmane-mx.org; Tue, 11 Oct 2022 09:16:26 +0200 Original-Received: from localhost ([::1]:49822 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oi9V1-0007Vy-7n for ged-emacs-devel@m.gmane-mx.org; Tue, 11 Oct 2022 03:16:23 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:43202) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oi98g-0003AW-EZ for emacs-devel@gnu.org; Tue, 11 Oct 2022 02:53:18 -0400 Original-Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:54637) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oi98e-0004vP-3t for emacs-devel@gnu.org; Tue, 11 Oct 2022 02:53:18 -0400 Original-Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id A105832000EB for ; Tue, 11 Oct 2022 02:53:12 -0400 (EDT) Original-Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 11 Oct 2022 02:53:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= secure.kjonigsen.net; h=cc:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:reply-to:sender:subject:subject:to:to; s=fm2; t= 1665471192; x=1665557592; bh=waVdhGbvnm8VARLVnnwzHbsAUjVUUpqvxq5 48o/WMsQ=; b=P5GCTwcPSr96awk4scPhGrn6KkNOG532BpTaFmFliIUJhhwJytz 2YMeRnx0WqSTdfzuINH+/6Qfv0IXhwH/75JENndr3MNPwDe+NSMszwyoZAOCxDah 771q3awI7xV74uuD+HSuaCrXbpcIZGZFE3FH3FCc91ba6689ey/4fu5HOBOsY1dL 9WwJb7DwnD4kNmvAkA00EY5TGTfTMxtUa22sbEyDwibLeae5jswJznWYdFLgu8R9 ftFXtuAdFxpNaFeiQYmudOIDT247Rv/FQBp522Vtd/HwVjJ6Q1ik6TAEwBSvUNJU ztAOjUyqdqh91HG5vsJrQKSMZM1JUu4vVfA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; t=1665471192; x=1665557592; bh=waVdhGbvnm8VA RLVnnwzHbsAUjVUUpqvxq548o/WMsQ=; b=OBS65NvK0xO6sQeSGVH1NEdOxPqET qDj57pCxhX08dXlsZ2wA24i1ZAn8NNFb+HbysHF6e+jk7xBHMjhrEneALTdPTxEO YNxuAh4bxXIgVkfDm4qXfU26EfBy8mPuiPsX5n2HlkNujY0y7rpS7xaavtauscl2 fJ2c49VwPdDL+MtazDrsejJqBEZ5S/2zKxPuYq+/EywqXPCI9QoeE3GN/vxpaf1r Wmq6/cJEA4P9MkNNymaD8B3dwQuDFakcMYfjxov+uPAh6l05fy/fS+nMVuJtCXdy 53VSv50u293uLioW/kZxnQVg5Wy2VcolEm06ibJcCwj7qsT6Zr2+pKq4w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeejhedgudduudcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurheptgfkffggfghruffvfhfhjgesrg dtreertdefjeenucfhrhhomheplfhoshhtvghinhgpmfhjpphnihhgshgvnhcuoehjohhs thgvihhnsehsvggtuhhrvgdrkhhjohhnihhgshgvnhdrnhgvtheqnecuggftrfgrthhtvg hrnhepteefueelgeevteduheelteevleehvdfgteejgeehudehfeevkeeufefgffetieej necuffhomhgrihhnpehkjhhnihhgshgvnhdrnhhonecuvehluhhsthgvrhfuihiivgeptd enucfrrghrrghmpehmrghilhhfrhhomhepjhhoshhtvghinhesshgvtghurhgvrdhkjhho nhhighhsvghnrdhnvght X-ME-Proxy: Feedback-ID: ib2f84088:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 11 Oct 2022 02:53:11 -0400 (EDT) Content-Language: nb-NO In-Reply-To: <87o7uk5f5i.fsf@thornhill.no> Received-SPF: pass client-ip=64.147.123.20; envelope-from=jostein@secure.kjonigsen.net; helo=wout4-smtp.messagingengine.com X-Spam_score_int: -46 X-Spam_score: -4.7 X-Spam_bar: ---- X-Spam_report: (-4.7 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, NICE_REPLY_A=-2.007, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:297432 Archived-At: This is a multi-part message in MIME format. --------------Wk0HxuiIoSON3WtwBOIEUiZn Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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. -- Jostein On 10.10.2022 09:08, Theodor Thornhill 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? > > > Thanks, see attached patch: > -- Vennlig hilsen *Jostein Kjønigsen* jostein@kjonigsen.net 🍵 jostein@gmail.com https://jostein.kjønigsen.no --------------Wk0HxuiIoSON3WtwBOIEUiZn Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

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.

--
Jostein

On 10.10.2022 09:08, Theodor Thornhill 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?


Thanks, see attached patch:
--
Vennlig hilsen
Jostein Kjønigsen

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no
--------------Wk0HxuiIoSON3WtwBOIEUiZn--