From: Vincenzo Pupillo <v.pupillo@gmail.com>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 71380@debbugs.gnu.org
Subject: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 11:04:00 +0200 [thread overview]
Message-ID: <2169161.yiUUSuA9gR@3-191.divsi.unimi.it> (raw)
In-Reply-To: <jwvsexqtccv.fsf-monnier+emacs@gnu.org>
Hi Stefan
In data giovedì 6 giugno 2024 16:06:21 CEST, Stefan Monnier ha scritto:
> 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))))
>
> ?
Yes is better. The code above is a copy of c-ts-mode--indent-style-setter.
It seemed too complicated to me too, but since it had been used there
I thought there was some reason.
>
> > +(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.
>
Done
> > +(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?
>
Yes, the first part is a literal copy of c-ts-mode--syntax-table.
java-ts-mode does exactly the same thing, so I thought it best
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))
>
> Is `comment-start` important enough to merit being part of the mode name?
>
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}"
>
> \\{php-ts-mode-map} is not necessary because `define-derived-mode` adds
> for you anyway.
>
Ok, done.
> > +(derived-mode-add-parents 'php-ts-mode '(php-mode))
>
> I'd move it next to the `define-derived-mode`.
>
>
Ok, done.
> Stefan
>
>
Thank you Stefan.
Vincenzo
next prev parent reply other threads:[~2024-06-07 9:04 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
2024-06-07 9:04 ` Vincenzo Pupillo [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2169161.yiUUSuA9gR@3-191.divsi.unimi.it \
--to=v.pupillo@gmail.com \
--cc=71380@debbugs.gnu.org \
--cc=monnier@iro.umontreal.ca \
/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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.