From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#61996: 30.0.50; Submitting elixir-ts-mode and heex-ts-mode Date: Sat, 11 Mar 2023 11:16:08 +0200 Message-ID: <83356by7fr.fsf@gnu.org> References: <87mt4qibnk.fsf@gmail.com> <83cz5m8515.fsf@gnu.org> <874jqx3h47.fsf@gmail.com> <83lek97mm1.fsf@gnu.org> <877cvtfz37.fsf@gmail.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="4505"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 61996@debbugs.gnu.org, theo@thornhill.no, casouri@gmail.com To: Wilhelm Kirschbaum Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Mar 11 10:17:24 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1pavLw-0000xf-1a for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 11 Mar 2023 10:17:24 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pavLc-00076T-EG; Sat, 11 Mar 2023 04:17:04 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pavLa-00076A-GB for bug-gnu-emacs@gnu.org; Sat, 11 Mar 2023 04:17:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pavLa-0001Pz-86 for bug-gnu-emacs@gnu.org; Sat, 11 Mar 2023 04:17:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pavLa-0007KH-4S for bug-gnu-emacs@gnu.org; Sat, 11 Mar 2023 04:17:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 11 Mar 2023 09:17:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 61996 X-GNU-PR-Package: emacs Original-Received: via spool by 61996-submit@debbugs.gnu.org id=B61996.167852619228117 (code B ref 61996); Sat, 11 Mar 2023 09:17:02 +0000 Original-Received: (at 61996) by debbugs.gnu.org; 11 Mar 2023 09:16:32 +0000 Original-Received: from localhost ([127.0.0.1]:56570 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pavL6-0007JR-9z for submit@debbugs.gnu.org; Sat, 11 Mar 2023 04:16:32 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:40232) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pavL4-0007J2-7u for 61996@debbugs.gnu.org; Sat, 11 Mar 2023 04:16:30 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pavKy-0001MG-L0; Sat, 11 Mar 2023 04:16:24 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=SCLEDybZh5y2IAdyQTUAxuzMy++ogfBmZLi8d1Kvqrc=; b=f5FuRgy7SQaV 7EoeHWnm26s/LbiHESreyv18fvfOgT2SoMRsjS6eQCzgZJikdw/CpA0CzSuvxT/6sX+okABJy3bXA HvSpAKQBx1jK6g+48F5NwHL6tGw83s+YW87sywzs8E4jnVLfcZaS5RH0B3+WxbAxRo/9heBK1VFxJ UE2BOdON1t1YLWgjhZQxXhE8QeXWwmWy4ig6iY+X45boS8gwoHUhMVITlyWdbPXkfn6DYWCpzkgPu Kvigb3Gisi3u4do63xaVA5ZZmvU0HAYzRXzjZvxBel288DtKiAOOzkYPt6EZBeTqhORScChKzhTSV 8AneteVMJ0mn6VWEaqHS8Q==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pavKy-0008GP-44; Sat, 11 Mar 2023 04:16:24 -0500 In-Reply-To: <877cvtfz37.fsf@gmail.com> (message from Wilhelm Kirschbaum on Mon, 06 Mar 2023 21:24:11 +0200) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:257768 Archived-At: > From: Wilhelm Kirschbaum > Cc: 61996@debbugs.gnu.org, casouri@gmail.com, theo@thornhill.no > Date: Mon, 06 Mar 2023 21:24:11 +0200 > > For this release it will be good just to get the basics to work as > is, but good to know it is an option. > > Attached are the updated patches. Thanks, a few minor comments below. > >From 88c941067da0e34e1e9ababeb813ba51378ae2cc Mon Sep 17 00:00:00 2001 > From: Wilhelm H Kirschbaum > Date: Mon, 6 Mar 2023 21:18:04 +0200 > Subject: [PATCH 1/2] Add heex-ts-mode > > --- > lisp/progmodes/heex-ts-mode.el | 185 ++++++++++++++++++ > .../heex-ts-mode-resources/indent.erts | 47 +++++ > test/lisp/progmodes/heex-ts-mode-tests.el | 9 + > 3 files changed, 241 insertions(+) > create mode 100644 lisp/progmodes/heex-ts-mode.el > create mode 100644 test/lisp/progmodes/heex-ts-mode-resources/indent.erts > create mode 100644 test/lisp/progmodes/heex-ts-mode-tests.el Please accompany the changes with a commit log message according to our conventions (see CONTRIBUTE for the conventions; search for "ChangeLog" there). In this case, just "New file" log should be sufficient for the new files you add. > +(declare-function treesit-parser-create "treesit.c") > +(declare-function treesit-node-child "treesit.c") > +(declare-function treesit-node-type "treesit.c") > +(declare-function treesit-install-language-grammar "treesit.el") AFAICS, the code uses more functions from treesit.c; please add declare-function forms for all of them , to avoid compilation warnings n systems where Emacs was built without tree-sitter. > +(defun heex-ts-mode--forward-sexp (&optional arg) > + (interactive "^p") Why is a command an internal function? That is unusual, as commands are by definition public. It looks like you thought the double-hyphen "--" notation is a simple delimiter between the package-name part of the symbol name and the rest? If so, you were mistaken: the double-hyphen means this is an internal function/variable. Please review all your symbol names in this patch and rename as appropriate. Btw, there's no need to have the prefix be the full name of the package, as in "elixir-ts-mode-". You could use "elixir-ts-" instead. > +;;;###autoload > +(define-derived-mode heex-ts-mode html-mode "Heex" html-mode? not html-ts-mode? > >From d13c34ed951e3e6fa473cd1bc2e955e20455022b Mon Sep 17 00:00:00 2001 > From: Wilhelm H Kirschbaum > Date: Mon, 6 Mar 2023 21:18:35 +0200 > > --- > lisp/progmodes/elixir-ts-mode.el | 626 ++++++++++++++++++ > .../elixir-ts-mode-resources/indent.erts | 147 ++++ > test/lisp/progmodes/elixir-ts-mode-tests.el | 31 + > 3 files changed, 804 insertions(+) > create mode 100644 lisp/progmodes/elixir-ts-mode.el > create mode 100644 test/lisp/progmodes/elixir-ts-mode-resources/indent.erts > create mode 100644 test/lisp/progmodes/elixir-ts-mode-tests.el Likewise here: please add a commit log message describing the changes. > +(declare-function treesit-parser-create "treesit.c") > +(declare-function treesit-node-child "treesit.c") > +(declare-function treesit-node-type "treesit.c") > +(declare-function treesit-node-child-by-field-name "treesit.c") > +(declare-function treesit-parser-language "treesit.c") > +(declare-function treesit-parser-included-ranges "treesit.c") > +(declare-function treesit-parser-list "treesit.c") > +(declare-function treesit-node-parent "treesit.c") > +(declare-function treesit-node-start "treesit.c") > +(declare-function treesit-query-compile "treesit.c") > +(declare-function treesit-install-language-grammar "treesit.el") Please verify that you have declare-function for all the functions from treesit.c this package uses, and only for those. > +(defgroup elixir-ts nil > + "Major mode for editing Ruby code." ^^^^ "Ruby"? > +;; used to distinguish from comment-face in query match Comments should be complete sentences: start with a capital letter and end with a period (here and elsewhere in the patches). > +(defface elixir-ts-font-comment-doc-identifier-face > + '((t (:inherit font-lock-doc-face))) > + "For use with @comment.doc tag.") This doc string is too terse. Imagine someone looking at it in a long list of symbols, not necessarily all of them faces. So something like this is better: Face used for @comment.doc tags in Elixir files. Likewise for other faces in the patch. > + (modify-syntax-entry ?@ "'" table) > + table) > + "Syntax table for `elixir-ts-mode.") ^ The closing ' quote is missing there.