From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Code quality of some -ts-mode major modes Date: Fri, 17 Mar 2023 10:08:52 +0000 Message-ID: <87fsa3g05n.fsf@posteo.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="35564"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Yuan Fu To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Fri Mar 17 11:09:30 2023 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 1pd71e-000920-LN for ged-emacs-devel@m.gmane-mx.org; Fri, 17 Mar 2023 11:09:30 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pd70n-0006kO-0T; Fri, 17 Mar 2023 06:08:37 -0400 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 1pd70l-0006k8-4t for emacs-devel@gnu.org; Fri, 17 Mar 2023 06:08:35 -0400 Original-Received: from mout01.posteo.de ([185.67.36.65]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pd70i-0007Ia-Og for emacs-devel@gnu.org; Fri, 17 Mar 2023 06:08:34 -0400 Original-Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 422622402FA for ; Fri, 17 Mar 2023 11:08:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1679047710; bh=WHGGrGygphE8pVhyvxd/9XpgRuyQzxRLxsNxlKzFD7E=; h=From:To:Cc:Subject:Date:From; b=Yna7WlwjBVzwqEyJwo8rkX40I/ImCaGiXnM2fkWwUHePJ6Z8HC5j2V+hkSyaPTrvC g/EFrYnK/9FvpOXbtpTbuBalkAb+B0a5t4ebQrneJj/ksNZtpSkhXJkruD/u13d5z4 mKZ8JtPY5dnPVJHnG1fiqzzVoEdJ+bdlda6DxIwq2teB2p4+7kN9MA6wH213Of2AtP ctIQbbNKK35KL3UNN3H+gXHnqzOcPfYQWDKzmv+7ykingLzD0+dALqu4UwBAZ8grbs 5PS6q3sZYMufXdKh/Zl24CUM7nnGWR7djNvcwrVOPQh507MS7rXkCRB7Gm/Ds65d9v a6rMDtnCxFAlQ== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4PdKb05BnXz9rxG; Fri, 17 Mar 2023 11:08:28 +0100 (CET) Received-SPF: pass client-ip=185.67.36.65; envelope-from=philipk@posteo.net; helo=mout01.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=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-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:304541 Archived-At: --=-=-= Content-Type: text/plain Hi, I took a look at some of the new tree-sitter major modes and was surprised at what I saw. Without meaning to belittle anyone, there were some basic "stylistic mistakes" that I wouldn't have expected to have gotten merged. I didn't look up the exact chronology, but it seems like there has been a lot of uncritical copying between these files. Take `yaml-ts-mode' for example. I attach a diff with a few changes that I would have expected to have been made before the file was merged: --=-=-= Content-Type: text/x-diff Content-Disposition: inline diff --git a/lisp/textmodes/yaml-ts-mode.el b/lisp/textmodes/yaml-ts-mode.el index dfa8d22fb34..43e8868fc44 100644 --- a/lisp/textmodes/yaml-ts-mode.el +++ b/lisp/textmodes/yaml-ts-mode.el @@ -2,10 +2,10 @@ ;; Copyright (C) 2022-2023 Free Software Foundation, Inc. -;; Author : Randy Taylor -;; Maintainer : Randy Taylor -;; Created : December 2022 -;; Keywords : yaml languages tree-sitter +;; Author: Randy Taylor +;; Maintainer: Randy Taylor +;; Created: December 2022 +;; Keywords: languages ;; This file is part of GNU Emacs. @@ -23,15 +23,18 @@ ;; along with GNU Emacs. If not, see . ;;; Commentary: -;; + +;; This file provides basic YAML syntax highlighting using Tree +;; Sitter. To use the `yaml-ts-mode' major mode you will have to make +;; sure that you have installed the appropriate grammar. ;;; Code: (require 'treesit) -(declare-function treesit-parser-create "treesit.c") +;; (declare-function treesit-parser-create "treesit.c") ;doesn't appear necessary -(defvar yaml-ts-mode--syntax-table +(defvar yaml-ts-mode-syntax-table (let ((table (make-syntax-table))) (modify-syntax-entry ?# "<" table) (modify-syntax-entry ?\n ">" table) @@ -59,7 +62,8 @@ yaml-ts-mode--font-lock-settings (null_scalar) (reserved_directive) (tag_directive) - (yaml_directive)] @font-lock-constant-face) + (yaml_directive)] + @font-lock-constant-face) :language 'yaml :feature 'delimiter @@ -83,7 +87,8 @@ yaml-ts-mode--font-lock-settings '([(block_scalar) (double_quote_scalar) (single_quote_scalar) - (string_scalar)] @font-lock-string-face) + (string_scalar)] + @font-lock-string-face) :language 'yaml :feature 'escape-sequence @@ -120,10 +125,9 @@ yaml-ts-mode--font-lock-settings ;;;###autoload (define-derived-mode yaml-ts-mode text-mode "YAML" "Major mode for editing YAML, powered by tree-sitter." - :group 'yaml - :syntax-table yaml-ts-mode--syntax-table + ;; :group 'yaml ;; no such customisation group was defined? - (when (treesit-ready-p 'yaml) + (when (treesit-ready-p 'yaml) ;why not raise an `user-error'? (treesit-parser-create 'yaml) ;; Comments. @@ -143,9 +147,8 @@ yaml-ts-mode (treesit-major-mode-setup))) -(if (treesit-ready-p 'yaml) - (add-to-list 'auto-mode-alist '("\\.ya?ml\\'" . yaml-ts-mode))) +(when (treesit-ready-p 'yaml) + (add-to-list 'auto-mode-alist '("\\.ya?ml\\'" . yaml-ts-mode))) (provide 'yaml-ts-mode) - ;;; yaml-ts-mode.el ends here --=-=-= Content-Type: text/plain In particular: The lack of a commentary section or any indication/pointer on how to install the grammar which is the necessary prequisite for the mode to have any effect to begin with (my understanding is that Emacs will not ship with these files, nor are any distributions working on providing them, right?). I considered mentioning the new command `treesit-install-language-grammar', but that seems pointless as long as `treesit-language-source-alist' is empty by default. My question: Would there be any objection from those involves with tree-sitter against me making changes like the ones I gave above? Maintaining some basic style in the core seems desirable to me, as we have seen that these files often serve as a template for creating new major modes. --=-=-=--