From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Vincenzo Pupillo <v.pupillo@gmail.com>
Cc: 71380@debbugs.gnu.org
Subject: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Thu, 06 Jun 2024 10:06:21 -0400 [thread overview]
Message-ID: <jwvsexqtccv.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <3686989.dWV9SEqChM@3-191.divsi.unimi.it> (Vincenzo Pupillo's message of "Wed, 05 Jun 2024 15:59:20 +0200")
I have not read the whole code, but just noticed the things below:
> + (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))))))))
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:
(dolist (buffer (buffer-list))
(with-current-buffer buffer
(when (derived-mode-p 'php-ts-mode)
(php-ts-mode-set-style val))))
?
> +(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 projects.
> +`WordPress' - use coding styles preferred for working with WordPress projects.
> +`Symfony' - use coding styles preferred for working with Symfony projects.
> +`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)
The :safe arg is also a function, so please #' it as well.
> +(defvar php-ts-mode--syntax-table
> + (let ((table (make-syntax-table)))
> + ;; Taken from the cc-langs version
Does this mean it comes from "the cc-mode-based `php-mode.el`" or from
`cc-langs.el` (and if so, which part, exactly)?
> +;; taken from c-ts-mode
[...]
> +;; taken from c-ts-mode
Are these literal copies?
Maybe we should consolidate the code with that of `c-ts-mode` to avoid
the code duplication?
> + (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))
Is `comment-start` important enough to merit being part of the mode name?
> +(define-derived-mode php-ts-mode prog-mode "PHP"
> + "Major mode for editing PHP, powered by tree-sitter.
> +
> +\\{php-ts-mode-map}"
\\{php-ts-mode-map} is not necessary because `define-derived-mode` adds
for you anyway.
> +(derived-mode-add-parents 'php-ts-mode '(php-mode))
I'd move it next to the `define-derived-mode`.
Stefan
next prev parent reply other threads:[~2024-06-06 14:06 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-05 13:59 bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php Vincenzo Pupillo
2024-06-06 6:58 ` Eli Zaretskii
2024-06-07 10:45 ` Vincenzo Pupillo
2024-06-07 11:12 ` Eli Zaretskii
2024-06-07 12:50 ` Vincenzo Pupillo
2024-06-07 13:44 ` Eli Zaretskii
2024-06-07 15:05 ` Vincenzo Pupillo
2024-06-08 9:31 ` Vincenzo Pupillo
2024-06-08 10:45 ` Eli Zaretskii
2024-06-08 11:15 ` Vincenzo Pupillo
2024-06-09 13:54 ` Eli Zaretskii
2024-06-09 17:23 ` Vincenzo Pupillo
2024-06-09 17:49 ` Eli Zaretskii
2024-06-09 19:37 ` Vincenzo Pupillo
2024-06-09 20:36 ` Vincenzo Pupillo
2024-06-12 9:25 ` Vincenzo Pupillo
2024-06-12 18:27 ` Eli Zaretskii
2024-06-12 20:44 ` Vincenzo Pupillo
2024-06-09 13:53 ` Eli Zaretskii
2024-06-06 14:06 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2024-06-07 9:04 ` Vincenzo Pupillo
2024-06-07 12:53 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-07 13:25 ` Vincenzo Pupillo
2024-06-07 13:49 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-07 14:37 ` Vincenzo Pupillo
2024-06-06 14:54 ` Andrea Corallo
2024-06-07 8:36 ` Vincenzo Pupillo
2024-06-07 13:39 ` Andrea Corallo
2024-06-07 17:02 ` Vincenzo Pupillo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvsexqtccv.fsf-monnier+emacs@gnu.org \
--to=bug-gnu-emacs@gnu.org \
--cc=71380@debbugs.gnu.org \
--cc=monnier@iro.umontreal.ca \
--cc=v.pupillo@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).