From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Vincenzo Pupillo Newsgroups: gmane.emacs.bugs Subject: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php Date: Fri, 07 Jun 2024 11:04:00 +0200 Message-ID: <2169161.yiUUSuA9gR@3-191.divsi.unimi.it> References: <3686989.dWV9SEqChM@3-191.divsi.unimi.it> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="20929"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 71380@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Jun 07 11:06:10 2024 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 1sFVY0-0005Gc-FY for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 07 Jun 2024 11:06:08 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sFVXg-0002KU-Rr; Fri, 07 Jun 2024 05:05:48 -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 1sFVXf-0002GS-I0 for bug-gnu-emacs@gnu.org; Fri, 07 Jun 2024 05:05:47 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1sFVXf-0003hQ-8f for bug-gnu-emacs@gnu.org; Fri, 07 Jun 2024 05:05:47 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1sFVXu-00084r-AY for bug-gnu-emacs@gnu.org; Fri, 07 Jun 2024 05:06:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Vincenzo Pupillo Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 07 Jun 2024 09:06:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 71380 X-GNU-PR-Package: emacs Original-Received: via spool by 71380-submit@debbugs.gnu.org id=B71380.171775112630942 (code B ref 71380); Fri, 07 Jun 2024 09:06:02 +0000 Original-Received: (at 71380) by debbugs.gnu.org; 7 Jun 2024 09:05:26 +0000 Original-Received: from localhost ([127.0.0.1]:37855 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sFVXJ-00082z-Oj for submit@debbugs.gnu.org; Fri, 07 Jun 2024 05:05:26 -0400 Original-Received: from mail-ej1-f44.google.com ([209.85.218.44]:51503) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sFVXH-00082X-LP for 71380@debbugs.gnu.org; Fri, 07 Jun 2024 05:05:24 -0400 Original-Received: by mail-ej1-f44.google.com with SMTP id a640c23a62f3a-a68b54577aaso239348966b.3 for <71380@debbugs.gnu.org>; Fri, 07 Jun 2024 02:05:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717751043; x=1718355843; darn=debbugs.gnu.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=A2fQE5/rBf4BK0Pm+yyF/zdQPxxQCVhrOx3MyqLd6VE=; b=YV7dKn/dNT1jtVBM9p4gj/jpO/vZeKkFUPNog1B+NVnpHa1O3tQcg2Z1gKBBabprO6 DeQaDeAQOu+TGI03sTG00nxKGBZzVb4hcn/PYRDRpoWp75sT79m9S5d0b5biIScB0Nya H2vaUb7DChhcJflAFU5D8LPgD364RQfqJphuj17hofQW9O8RGJeI4EQ3Fynjx3Xg6woD GIVlQOqSBILxWhSwZheT/AbLnn5Q7+v3XV1HhPLy8nNCCNCMyI8z2VHWP4uBONvBOYHS M2CHKeexS8GEiMTvnF2DmCRODlGe2ux3kfcESe4yyCcyLAMYv0tBnGPJnFIYNSXiJm2Z 87Cg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717751043; x=1718355843; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=A2fQE5/rBf4BK0Pm+yyF/zdQPxxQCVhrOx3MyqLd6VE=; b=WqnQspGgcewII2Xuu7TCnGfMONGqQ8mLD46lU8WzyBb1B6FLNatypMOf/AOfwMzI0m +OucBpTnv/zQ/36TYckmrHs2liy8etej5KyngMPQzHu0zZg+cvlcvUBMTrMZtCYHvkkw GTuW14ZqLUT/8g5ERmVi6FAsTELPlvzhP0dqnWpMpDGG3tqUzJdwtUR7mFIBc3cp/kwq xHQt2wp9QrH5ZooFR11vzYRRqqAYASVD6DS7+Y3ObtWJt0KW08TlxZggoxcZyWlumwza E/tnAVD36rfvDwtlS1uPM/F0kBBftzUqIX2BtLMSaqkVLzTstFDldkgH4aaZgSgu28F9 6gRA== X-Gm-Message-State: AOJu0YwRuKGRO0+SzB8COUZZq3UIe7xWULp0cxdtmu9PQN4b0qR0oXhg u/XgsQpbqdbEP6LcqYE89mLfCSlVL6rkwlOE1x9M1sDRc6wnECtr X-Google-Smtp-Source: AGHT+IGR9tlLFK7QRnmM3I6WEfFT1DuOTWNGvDstYxveXhEqL0xYwEpyfCPCBfwfyT/zEdnRazhFBA== X-Received: by 2002:a17:907:3e8b:b0:a68:c744:725b with SMTP id a640c23a62f3a-a6cd665f1e1mr150138366b.32.1717751042409; Fri, 07 Jun 2024 02:04:02 -0700 (PDT) Original-Received: from 3-191.divsi.unimi.it (3-191.divsi.unimi.it. [159.149.3.191]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a6c8070e1c1sm212735166b.168.2024.06.07.02.04.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 07 Jun 2024 02:04:01 -0700 (PDT) In-Reply-To: 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:286752 Archived-At: Hi Stefan In data gioved=C3=AC 6 giugno 2024 16:06:21 CEST, Stefan Monnier ha scritto: > I have not read the whole code, but just noticed the things below: >=20 > > + (named-let loop ((res nil) > > + (buffers (buffer-list))) > > + (if (null buffers) > > + (mapc (lambda (b) > > + (with-current-buffer b > > + (php-ts-mode-set-style val))) > > + res) > > + (let ((buffer (car buffers))) > > + (with-current-buffer buffer > > + (if (derived-mode-p 'php-ts-mode) > > + (loop (append res (list buffer)) (cdr buffers)) > > + (loop res (cdr buffers)))))))) >=20 > These `loop` calls are not in tail-position, so this will eat up the > stack (because of the surrounding `with-current-buffer`). > Also, I don't understand why you do it in such a complicated way. > Isn't this better written: >=20 > (dolist (buffer (buffer-list)) > (with-current-buffer buffer > (when (derived-mode-p 'php-ts-mode) > (php-ts-mode-set-style val)))) >=20 > ? Yes is better. The code above is a copy of c-ts-mode--indent-style-setter.= =20 It seemed too complicated to me too, but since it had been used there=20 I thought there was some reason. >=20 > > +(defcustom php-ts-mode-indent-style 'psr2 > > + "Style used for indentation. > > +The selected style could be one of: > > +`PSR-2/PSR-12' - use PSR standards (PSR-2, PSR-12), thi is the default. > > +`PEAR' - use coding styles preferred for PEAR code and modules. > > +`Drupal' - use coding styles preferred for working with Drupal project= s. > > +`WordPress' - use coding styles preferred for working with WordPress p= rojects. > > +`Symfony' - use coding styles preferred for working with Symfony proje= cts. > > +`Zend' - use coding styles preferred for working with Zend projects. > > + > > +If one of the supplied styles doesn't suffice, a function could be > > +set instead. This function is expected return a list that > > +follows the form of `treesit-simple-indent-rules'." > > + :tag "PHP indent style" > > + :version "30.1" > > + :type '(choice (const :tag "PSR-2/PSR-12" psr2) > > + (const :tag "PEAR" pear) > > + (const :tag "Drupal" drupal) > > + (const :tag "WordPress" wordpress) > > + (const :tag "Symfony" symfony) > > + (const :tag "Zend" zend) > > + (function :tag "A function for user customized style" ignore)) > > + :set #'php-ts-mode--indent-style-setter > > + :safe 'c-ts-indent-style-safep) >=20 > The :safe arg is also a function, so please #' it as well. >=20 Done > > +(defvar php-ts-mode--syntax-table > > + (let ((table (make-syntax-table))) > > + ;; Taken from the cc-langs version >=20 > Does this mean it comes from "the cc-mode-based `php-mode.el`" or from > `cc-langs.el` (and if so, which part, exactly)? >=20 > > +;; taken from c-ts-mode > [...] > > +;; taken from c-ts-mode >=20 > Are these literal copies? > Maybe we should consolidate the code with that of `c-ts-mode` to avoid > the code duplication? >=20 Yes, the first part is a literal copy of c-ts-mode--syntax-table.=20 java-ts-mode does exactly the same thing, so I thought it best=20 to avoid depending on c-ts-mode--syntax-table. > > + (cond > > + ((equal comment-start "/*") (setq-local comment-end "*/")) > > + ((equal comment-start "//") (setq-local comment-end "")) > > + ((equal comment-start "#") (setq-local comment-end "")) > > + ((equal comment-start "/**") (setq-local comment-end "*/"))) > > + (setq mode-name (concat "PHP" (string-trim-right comment-start))) > > + (force-mode-line-update)) >=20 > Is `comment-start` important enough to merit being part of the mode name? >=20 Sorry. I didn't understand. Could you please clarify? > > +(define-derived-mode php-ts-mode prog-mode "PHP" > > + "Major mode for editing PHP, powered by tree-sitter. > > + > > +\\{php-ts-mode-map}" >=20 > \\{php-ts-mode-map} is not necessary because `define-derived-mode` adds > for you anyway. >=20 Ok, done. > > +(derived-mode-add-parents 'php-ts-mode '(php-mode)) >=20 > I'd move it next to the `define-derived-mode`. >=20 >=20 Ok, done. > Stefan >=20 >=20 Thank you Stefan. Vincenzo