GNU bug report logs - #71380
30.0.50; Submitting php-ts-mode, new major mode for php

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71380 in the body.
You can then email your comments to 71380 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Wed, 05 Jun 2024 15:59:20 +0200

[Message part 1 (text/plain, inline)]

Hi, 
I would like to submit php-ts-mode. 
This major mode this major mode, in addition to font-lock for PHP implements the following features:
* font-lock for html, javascript, css and phpdoc.
* six different indentation styles (PSR, PEAR, Zend, Drupal, Wordpress, Symfony).
* Imenu
* Flymake
* Which-function
* a helper function to simplify the installation of parsers, in versions used to develop major-mode
* PHP built-in server support
* Shell interaction: execute PHP code in an inferior PHP process.


I completed the assignment process in March 2023.
Thank you.

Vincenzo

[0001-Add-php-ts-mode.patch (text/x-patch, attachment)]

Message #8 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Philip Kaludercic <philipk <at> posteo.net>, Yuan Fu <casouri <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Thu, 06 Jun 2024 09:58:11 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Date: Wed, 05 Jun 2024 15:59:20 +0200
> 
> I would like to submit php-ts-mode. 
> This major mode this major mode, in addition to font-lock for PHP implements the following features:
> * font-lock for html, javascript, css and phpdoc.
> * six different indentation styles (PSR, PEAR, Zend, Drupal, Wordpress, Symfony).
> * Imenu
> * Flymake
> * Which-function
> * a helper function to simplify the installation of parsers, in versions used to develop major-mode
> * PHP built-in server support
> * Shell interaction: execute PHP code in an inferior PHP process.

Thanks, I added Stefan, Philip and Yuan to the discussion, in case they have
comments.

> +---
> +*** New major mode 'php-ts-mode'.
> +A major mode based on the tree-sitter library for editing

This seems to be an incomplete sentence.

Also, I think we should add PHP to the list of modes in the "Program
Modes" node of the Emacs user manual.

> +(defun php-ts-mode-install-parsers ()
> +  "Install all the required treesitter parser.
                                          ^^^^^^
"parsers", in plural.

> +`php-ts-mode--language-source-alist' define which parsers to install."
                                        ^^^^^^
"defines".

> +(defcustom php-ts-mode-indent-offset 4
> +  "Number of spaces for each indentation step (default) in `php-ts-mode'."
                ^^^^^^
"columns", I guess?

And what does "(default)" mean here?

> +(defcustom php-ts-mode-js-css-indent-offset html-ts-mode-indent-offset
> +  "JavaScript and CSS indent spaces related to the <script> and <style> HTML tags.
> +By default, the value is the same as `html-ts-mode-indent-offset'"
                                                                    ^
Period missing there at the end of the sentence.

> +(defcustom php-ts-mode-php-executable (or (executable-find "php") "/usr/bin/php")
> +  "The location of PHP executable."
> +  :tag "PHP Executable"
> +  :version "30.1"
> +  :type 'string
           ^^^^^^^
Should this be 'file instead?

> +(defcustom php-ts-mode-php-config nil
> +  "The location of php.ini file.
> +If nil the default one is used to run the embedded webserver or
> +inferior PHP process."
> +  :tag "PHP Init file"
> +  :version "30.1"
> +  :type 'string

Likewise here.

> +(defcustom php-ts-mode-ws-document-root nil
> +  "The root of the documents that the PHP built-in webserver will serve.
> +If nil `php-ts-mode-run-php-webserver' will ask you for the document root."
> +  :tag "PHP built-in web server document root"
> +  :version "30.1"
> +  :type 'string

And this one perhaps should be 'directory?

> +(defun php-ts-mode--array-element-heuristic (node parent bol &rest _)
> +  "Return of the position of the first element of the array.

The "of" part should be deleted here, I think.

> +(defun php-ts-mode-run-php-webserver (port hostname document-root
> +					   &optional router-script num-of-workers)
> +  "Run the PHP Built-in web-server on a specified PORT.

This should mention the mandatory arguments.  How about

  Run PHP built-in web server on HOSTNAME:PORT to serve DOCUMENT-ROOT.

> +PORT: Port number of built-in web server, default `php-ts-mode-ws-port'.
> +If a default value is nil, the value is prompted.

Please avoid passive voice as much as possible.  In this case, I'd use

  Prompt for the port if the default value is nil.

Btw, it will prompt also if the value of the argument is nil, right?
So the above should say that as well.

> +HOSTNAME: Hostname or IP address of Built-in web server,
> +default `php-ts-mode-ws-hostname'.  If a default value is nil,
> +the value is prompted.
> +DOCUMENT-ROOT: Path to Document root, default `php-ts-mode-ws-document-root'.
> +If a default value is nil, the value is prompted.

Same comments for these two arguments.

> +When called with \\[universal-argument] it requires PORT,
> +HOSTNAME, DOCUMENT-ROOT and ROUTER-SCRIPT."

Our style of saying that is like this:

  Interactively, when invoked with prefix argument, always prompt
  for PORT, HOSTNAME, DOCUMENT-ROOT and ROUTER-SCRIPT.

> +    (cond (num-of-workers (setenv "PHP_CLI_SERVER_WORKERS" num-of-workers))
> +	  (php-ts-mode-ws-workers
> +	   (setenv "PHP_CLI_SERVER_WORKERS" php-ts-mode-ws-workers)))

setenv modifies process-environment for the entire duration of this
Emacs session.  If we only want to affect the following invocation of
make-comint, then perhaps let-binding process-environment around the
call to make-comint is a better way?

> +(defun run-php (&optional cmd config)
> +  "Run a PHP interpreter as a inferior process.
                               ^
"an"

> +Argumens CMD an CONFIG, defaults to `php-ts-mode-php-executable'
                           ^^^^^^^^
"default", in plural.  Also, I think it is better to say "which
default", given the rest of the sentence.

> +If CONFIG is nil the intepreter run with the default php.ini.
                                   ^^^
"runs", singular.

> +if `php-ts-mode-php-executable' is not defined the user is prompted."
   ^^                                             ^^^^^^^^^^^^^^^^^^^^
"If", capitalized.  And please use "prompt the user" to avoid the
passive voice.

> +(defun inferior-php-ts-mode-startup (cmd config)
> +  "Start an inferior PHP process.
> +CMD is the command to run, CONFIG, if not nil, is a php.ini file to use."

Again, please try to mention the mandatory arguments in the first
line of the doc string.  For example:

  Start an inferior PHP process with command CMD and init file CONFIG.

Message #11 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Thu, 06 Jun 2024 10:06:21 -0400

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

Message #14 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Andrea Corallo <acorallo <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Thu, 06 Jun 2024 10:54:12 -0400

Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:

> Hi, 
> I would like to submit php-ts-mode. 
> This major mode this major mode, in addition to font-lock for PHP implements the following features:
> * font-lock for html, javascript, css and phpdoc.
> * six different indentation styles (PSR, PEAR, Zend, Drupal, Wordpress, Symfony).
> * Imenu
> * Flymake
> * Which-function
> * a helper function to simplify the installation of parsers, in versions used to develop major-mode
> * PHP built-in server support
> * Shell interaction: execute PHP code in an inferior PHP process.
>
>
> I completed the assignment process in March 2023.
> Thank you.
>
> Vincenzo

Ciao Vincenzo,

applying the patch to the tree and building I see this warning is
introduced on my machine.

'Warning (treesit): Cannot activate tree-sitter, because language grammar for html is unavailable (not-found):'

Also, maybe you want to add ";; Maintainer:" as well with your name so
we know who to bother in case? :)

  Andrea

Message #17 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Andrea Corallo <acorallo <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 10:36:48 +0200

Ciao Andrea, 
I think the warning is due to html-ts-mode. It checks whether or not the parser exists, but even if it does not exist it tries to create it:
...

 (unless (treesit-ready-p 'html)
   (error “Tree-sitter for HTML isn't available”))

  (treesit-parser-create 'html)

....

I fixed it by replacing “unless” with “if” ...
Do I open another bug for this patch?

Ok for the ";; Maintainer". 
Go ahead and bother me! :D

V.

In data giovedì 6 giugno 2024 16:54:12 CEST, Andrea Corallo ha scritto:
> Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:
> 
> > Hi, 
> > I would like to submit php-ts-mode. 
> > This major mode this major mode, in addition to font-lock for PHP implements the following features:
> > * font-lock for html, javascript, css and phpdoc.
> > * six different indentation styles (PSR, PEAR, Zend, Drupal, Wordpress, Symfony).
> > * Imenu
> > * Flymake
> > * Which-function
> > * a helper function to simplify the installation of parsers, in versions used to develop major-mode
> > * PHP built-in server support
> > * Shell interaction: execute PHP code in an inferior PHP process.
> >
> >
> > I completed the assignment process in March 2023.
> > Thank you.
> >
> > Vincenzo
> 
> Ciao Vincenzo,
> 
> applying the patch to the tree and building I see this warning is
> introduced on my machine.
> 
> 'Warning (treesit): Cannot activate tree-sitter, because language grammar for html is unavailable (not-found):'
> 
> Also, maybe you want to add ";; Maintainer:" as well with your name so
> we know who to bother in case? :)
> 
>   Andrea
> 

Message #20 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 11:04:00 +0200

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

Message #23 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 12:45:05 +0200

[Message part 1 (text/plain, inline)]

Hi Eli, Thank you.

In data giovedì 6 giugno 2024 08:58:11 CEST, hai scritto:
> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > Date: Wed, 05 Jun 2024 15:59:20 +0200
> > 
> > I would like to submit php-ts-mode.
> > This major mode this major mode, in addition to font-lock for PHP
> > implements the following features: * font-lock for html, javascript, css
> > and phpdoc.
> > * six different indentation styles (PSR, PEAR, Zend, Drupal, Wordpress,
> > Symfony). * Imenu
> > * Flymake
> > * Which-function
> > * a helper function to simplify the installation of parsers, in versions
> > used to develop major-mode * PHP built-in server support
> > * Shell interaction: execute PHP code in an inferior PHP process.
> 
> Thanks, I added Stefan, Philip and Yuan to the discussion, in case they have
> comments.
> 
> > +---
> > +*** New major mode 'php-ts-mode'.
> > +A major mode based on the tree-sitter library for editing
> 
> This seems to be an incomplete sentence.
Done

> 
> Also, I think we should add PHP to the list of modes in the "Program
> Modes" node of the Emacs user manual.
> 
> > +(defun php-ts-mode-install-parsers ()
> > +  "Install all the required treesitter parser.
> 
>                                           ^^^^^^
> "parsers", in plural.
>
Done 
> > +`php-ts-mode--language-source-alist' define which parsers to install."
> 
>                                         ^^^^^^
> "defines".
> 
Done
> > +(defcustom php-ts-mode-indent-offset 4
> > +  "Number of spaces for each indentation step (default) in
> > `php-ts-mode'."
> 
>                 ^^^^^^
> "columns", I guess?
> 
> And what does "(default)" mean here?
> 
The comment is the same as c-ts-mode-indent-offset except for “(default)” , 
which I deleted

> > +(defcustom php-ts-mode-js-css-indent-offset html-ts-mode-indent-offset
> > +  "JavaScript and CSS indent spaces related to the <script> and <style>
> > HTML tags. +By default, the value is the same as
> > `html-ts-mode-indent-offset'"
>                                                                     ^
> Period missing there at the end of the sentence.
> 
Done 
> > +(defcustom php-ts-mode-php-executable (or (executable-find "php")
> > "/usr/bin/php") +  "The location of PHP executable."
> > +  :tag "PHP Executable"
> > +  :version "30.1"
> > +  :type 'string
> 
>            ^^^^^^^
> Should this be 'file instead?
> 
Done

> > +(defcustom php-ts-mode-php-config nil
> > +  "The location of php.ini file.
> > +If nil the default one is used to run the embedded webserver or
> > +inferior PHP process."
> > +  :tag "PHP Init file"
> > +  :version "30.1"
> > +  :type 'string
> 
> Likewise here.
>
Done
 
> > +(defcustom php-ts-mode-ws-document-root nil
> > +  "The root of the documents that the PHP built-in webserver will serve.
> > +If nil `php-ts-mode-run-php-webserver' will ask you for the document
> > root." +  :tag "PHP built-in web server document root"
> > +  :version "30.1"
> > +  :type 'string
> 
> And this one perhaps should be 'directory?
> 
Done

> > +(defun php-ts-mode--array-element-heuristic (node parent bol &rest _)
> > +  "Return of the position of the first element of the array.
> 
> The "of" part should be deleted here, I think.
>
I'm not sure how to explain it. Different indentation styles indent the 
elements of an array differently when written on multiple rows. For example.
in PSR2 it is like this:
$a = array("a" => 1,
     "b" => 2,
     "c" => 3);
while with Zend it is like this:
$a = array("a" => 1,
           "b" => 2,
           "c" => 3);
What do you suggest?

> > +(defun php-ts-mode-run-php-webserver (port hostname document-root
> > +					   &optional router-script num-
of-workers)
> > +  "Run the PHP Built-in web-server on a specified PORT.
> 
> This should mention the mandatory arguments.  How about
> 
>   Run PHP built-in web server on HOSTNAME:PORT to serve DOCUMENT-ROOT.
> 
Sorry, an error while merge my branch. all parameters are optional.

> > +PORT: Port number of built-in web server, default `php-ts-mode-ws-port'.
> > +If a default value is nil, the value is prompted.
> 
> Please avoid passive voice as much as possible.  In this case, I'd use
> 
>   Prompt for the port if the default value is nil.
> 
> Btw, it will prompt also if the value of the argument is nil, right?
> So the above should say that as well.
Yes, done.

> 
> > +HOSTNAME: Hostname or IP address of Built-in web server,
> > +default `php-ts-mode-ws-hostname'.  If a default value is nil,
> > +the value is prompted.
> > +DOCUMENT-ROOT: Path to Document root, default
> > `php-ts-mode-ws-document-root'. +If a default value is nil, the value is
> > prompted.
> 
> Same comments for these two arguments.
>
Ok, done.
 
> > +When called with \\[universal-argument] it requires PORT,
> > +HOSTNAME, DOCUMENT-ROOT and ROUTER-SCRIPT."
> 
> Our style of saying that is like this:
> 
>   Interactively, when invoked with prefix argument, always prompt
>   for PORT, HOSTNAME, DOCUMENT-ROOT and ROUTER-SCRIPT.
> 
Done

> > +    (cond (num-of-workers (setenv "PHP_CLI_SERVER_WORKERS"
> > num-of-workers)) +	  (php-ts-mode-ws-workers
> > +	   (setenv "PHP_CLI_SERVER_WORKERS" php-ts-mode-ws-workers)))
> 
> setenv modifies process-environment for the entire duration of this
> Emacs session.  If we only want to affect the following invocation of
> make-comint, then perhaps let-binding process-environment around the
> call to make-comint is a better way?
> 
Yes is better. I rewrote as a let variable:
(process-environment
          (cons (cond
                 (num-of-workers (format "PHP_CLI_SERVER_WORKERS=%d" num-of-
workers))
                 (php-ts-mode-ws-workers (format "PHP_CLI_SERVER_WORKERS=%d" 
php-ts-mode-ws-workers)))
                process-environment))

PHP accepts that environment variable only if > 1, 
otherwise it issues in warning.


> > +(defun run-php (&optional cmd config)
> > +  "Run a PHP interpreter as a inferior process.
> 
>                                ^
> "an"
> 
Done

> > +Argumens CMD an CONFIG, defaults to `php-ts-mode-php-executable'
> 
>                            ^^^^^^^^
> "default", in plural.  Also, I think it is better to say "which
> default", given the rest of the sentence.
> 
> > +If CONFIG is nil the intepreter run with the default php.ini.
> 
>                                    ^^^
> "runs", singular.
> 
Done

> > +if `php-ts-mode-php-executable' is not defined the user is prompted."
> 
>    ^^                                             ^^^^^^^^^^^^^^^^^^^^
> "If", capitalized.  And please use "prompt the user" to avoid the
> passive voice.
> 
> > +(defun inferior-php-ts-mode-startup (cmd config)
> > +  "Start an inferior PHP process.
> > +CMD is the command to run, CONFIG, if not nil, is a php.ini file to use."
> 
> Again, please try to mention the mandatory arguments in the first
> line of the doc string.  For example:
> 
>   Start an inferior PHP process with command CMD and init file CONFIG

Done


As I wrote to Andrea, the compilation warning seems to be caused 
by html-ts-mode. I have prepared another patch that fixes the problem.

Thank you.

Vincenzo

[0001-Add-php-ts-mode.patch (text/x-patch, attachment)]

[0001-Do-not-run-treesit-parser-create-if-HTML-parser-is-u.patch (text/x-patch, attachment)]

Message #26 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 14:12:25 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: 71380 <at> debbugs.gnu.org
> Date: Fri, 07 Jun 2024 12:45:05 +0200
> 
> > > +(defun php-ts-mode--array-element-heuristic (node parent bol &rest _)
> > > +  "Return of the position of the first element of the array.
> > 
> > The "of" part should be deleted here, I think.
> >
> I'm not sure how to explain it. Different indentation styles indent the 
> elements of an array differently when written on multiple rows. For example.
> in PSR2 it is like this:
> $a = array("a" => 1,
>      "b" => 2,
>      "c" => 3);
> while with Zend it is like this:
> $a = array("a" => 1,
>            "b" => 2,
>            "c" => 3);
> What do you suggest?

What does the function return in each of these two cases?

Message #29 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 14:50:24 +0200

In data venerdì 7 giugno 2024 13:12:25 CEST, Eli Zaretskii ha scritto:
> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > Cc: 71380 <at> debbugs.gnu.org
> > Date: Fri, 07 Jun 2024 12:45:05 +0200
> > 
> > > > +(defun php-ts-mode--array-element-heuristic (node parent bol &rest _)
> > > > +  "Return of the position of the first element of the array.
> > > 
> > > The "of" part should be deleted here, I think.
> > >
> > I'm not sure how to explain it. Different indentation styles indent the 
> > elements of an array differently when written on multiple rows. For example.
> > in PSR2 it is like this:
> > $a = array("a" => 1,
> >      "b" => 2,
> >      "c" => 3);
> > while with Zend it is like this:
> > $a = array("a" => 1,
> >            "b" => 2,
> >            "c" => 3);
> > What do you suggest?
> 
> What does the function return in each of these two cases?
> 
If '$a = array(' is on the same line as '“a” => 1,' it returns the initial position of '“a”, 
otherwise the starting position of 'array(', like PSR2. 
In terms of tree-sitter-php the first case is:
(treesit-node-start (treesit-node-child parent 2))
while the second is: parent indentation + offset.

Intricate, but can handle nested array declarations in styles like Zend. 
It is currently used only for Zend but I am starting to look at another style named PER (https://www.php-fig.org/per/coding-style/). 

Thank you.
Vincenzo

Message #32 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 08:53:04 -0400

>>     (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.

Aha!  Thanks, I guess we should check the
`c-ts-mode--indent-style-setter` situation, as well, then.

>> > +(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.

Hmm... that makes the comment hard to understand.

>> > +  (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?

You have:

    (setq mode-name (concat "PHP" (string-trim-right comment-start)))

which means the mode-line will display `comment-start` (along with the
usual other things).  Is it really a good idea, given how the mode-line
is already often "too full"?  Which other major mode does that?
What's special about `comment-start` to make it deserve this honor?


        Stefan

Message #35 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 15:25:16 +0200

In data venerdì 7 giugno 2024 14:53:04 CEST, Stefan Monnier ha scritto:
> >>     (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.
> 
> Aha!  Thanks, I guess we should check the
> `c-ts-mode--indent-style-setter` situation, as well, then.
> 
> >> > +(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.
> 
> Hmm... that makes the comment hard to understand.
> 
Yes. Perhaps it would be better to remove it.

> >> > +  (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?
> 
> You have:
> 
>     (setq mode-name (concat "PHP" (string-trim-right comment-start)))
> 
> which means the mode-line will display `comment-start` (along with the
> usual other things).  Is it really a good idea, given how the mode-line
> is already often "too full"?  Which other major mode does that?
> What's special about `comment-start` to make it deserve this honor?
> 
> 
c-ts-mode-set-modeline does the same, which is why I did it too. 


>         Stefan
> 
> 

Thanks

Vincenzo

Message #38 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Andrea Corallo <acorallo <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 09:39:04 -0400

Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:

> Ciao Andrea, 
> I think the warning is due to html-ts-mode. It checks whether or not the parser exists, but even if it does not exist it tries to create it:
> ...
>
>  (unless (treesit-ready-p 'html)
>    (error “Tree-sitter for HTML isn't available”))
>
>   (treesit-parser-create 'html)

What is not clear to me is why we should run this while compiling php-ts-mode.

> ....
>
> I fixed it by replacing “unless” with “if” ...
> Do I open another bug for this patch?

But in that case one could activate html-ts-mode even with no parser? 🤔

> Ok for the ";; Maintainer". 
> Go ahead and bother me! :D

:D

Thanks

  Andrea

Message #41 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 16:44:44 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: 71380 <at> debbugs.gnu.org
> Date: Fri, 07 Jun 2024 14:50:24 +0200
> 
> In data venerdì 7 giugno 2024 13:12:25 CEST, Eli Zaretskii ha scritto:
> 
> > > > > +(defun php-ts-mode--array-element-heuristic (node parent bol &rest _)
> > > > > +  "Return of the position of the first element of the array.
> > > > 
> > > > The "of" part should be deleted here, I think.
> > > >
> > > I'm not sure how to explain it. Different indentation styles indent the 
> > > elements of an array differently when written on multiple rows. For example.
> > > in PSR2 it is like this:
> > > $a = array("a" => 1,
> > >      "b" => 2,
> > >      "c" => 3);
> > > while with Zend it is like this:
> > > $a = array("a" => 1,
> > >            "b" => 2,
> > >            "c" => 3);
> > > What do you suggest?
> > 
> > What does the function return in each of these two cases?
> > 
> If '$a = array(' is on the same line as '“a” => 1,' it returns the initial position of '“a”, 
> otherwise the starting position of 'array(', like PSR2. 
> In terms of tree-sitter-php the first case is:
> (treesit-node-start (treesit-node-child parent 2))
> while the second is: parent indentation + offset.

Does it return a buffer position or a column?  You seem to say that
sometimes it returns the former and sometimes the latter.

Message #44 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 09:49:08 -0400

> c-ts-mode-set-modeline does the same, which is why I did it too. 

I see.  FWIW, I consider such things as errors: major mode should
provide  the functionality that depends on the language, but there's
nothing special about C comments that makes `comment-start` more
deserving of being in the mode-line in a C mode than in any other mode.

IOW, it seems like this is a case where the major mode author imposed
his own preference.  If you want `comment-start` in the mode-line in C,
then most likely you also want it in many/most other modes and so you
should implement a minor mode for that which would be usable in
any mode.


        Stefan

Message #47 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 16:37:33 +0200

> > c-ts-mode-set-modeline does the same, which is why I did it too. 
> 
> I see.  FWIW, I consider such things as errors: major mode should
> provide  the functionality that depends on the language, but there's
> nothing special about C comments that makes `comment-start` more
> deserving of being in the mode-line in a C mode than in any other mode.
> 
> IOW, it seems like this is a case where the major mode author imposed
> his own preference.  If you want `comment-start` in the mode-line in C,
> then most likely you also want it in many/most other modes and so you
> should implement a minor mode for that which would be usable in
> any mode.

Agreed. Deleted.

> 
> 
>         Stefan
> 
> 

Thank you Stefan.

Vincenzo

Message #50 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 17:05:58 +0200

In data venerdì 7 giugno 2024 15:44:44 CEST, Eli Zaretskii ha scritto:
> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > Cc: 71380 <at> debbugs.gnu.org
> > Date: Fri, 07 Jun 2024 14:50:24 +0200
> > 
> > In data venerdì 7 giugno 2024 13:12:25 CEST, Eli Zaretskii ha scritto:
> > 
> > > > > > +(defun php-ts-mode--array-element-heuristic (node parent bol &rest _)
> > > > > > +  "Return of the position of the first element of the array.
> > > > > 
> > > > > The "of" part should be deleted here, I think.
> > > > >
> > > > I'm not sure how to explain it. Different indentation styles indent the 
> > > > elements of an array differently when written on multiple rows. For example.
> > > > in PSR2 it is like this:
> > > > $a = array("a" => 1,
> > > >      "b" => 2,
> > > >      "c" => 3);
> > > > while with Zend it is like this:
> > > > $a = array("a" => 1,
> > > >            "b" => 2,
> > > >            "c" => 3);
> > > > What do you suggest?
> > > 
> > > What does the function return in each of these two cases?
> > > 
> > If '$a = array(' is on the same line as '“a” => 1,' it returns the initial position of '“a”, 
> > otherwise the starting position of 'array(', like PSR2. 
> > In terms of tree-sitter-php the first case is:
> > (treesit-node-start (treesit-node-child parent 2))
> > while the second is: parent indentation + offset.
> 
> Does it return a buffer position or a column?  You seem to say that
> sometimes it returns the former and sometimes the latter.
> 
The point in both cases. 

Vincenzo

Message #53 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Andrea Corallo <acorallo <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Fri, 07 Jun 2024 19:02:52 +0200

In data venerdì 7 giugno 2024 15:39:04 CEST, Andrea Corallo ha scritto:
> Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:
> > Ciao Andrea,
> > I think the warning is due to html-ts-mode. It checks whether or not the
> > parser exists, but even if it does not exist it tries to create it: ...
> > 
> >  (unless (treesit-ready-p 'html)
> >  
> >    (error “Tree-sitter for HTML isn't available”))
> >   
> >   (treesit-parser-create 'html)
> 
> What is not clear to me is why we should run this while compiling
> php-ts-mode.
I don't know. Maybe because there is a '(require 'html-ts-mode)' ?

> > ....
> > 
> > I fixed it by replacing “unless” with “if” ...
> > Do I open another bug for this patch?
> 
> But in that case one could activate html-ts-mode even with no parser? 🤔
> 
I don't think so. Because if the parser is not there, the rest of the code is 
not executed. I don't know the details, but all '*-ts modes' do this.

> > Ok for the ";; Maintainer".
> > Go ahead and bother me! :D
> :
> :D
> 
> Thanks
> 
>   Andrea

Ciao
Vincenzo

Message #56 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sat, 08 Jun 2024 11:31:05 +0200

[Message part 1 (text/plain, inline)]

Hi Eli, 
this is the patch update. I followed Stefan's advice and removed the 
modification of the 'mode-line' and the comment in the 'php-ts-mode--syntax-
table'. 
Thanks!

Vincenzo

In data venerdì 7 giugno 2024 17:05:58 CEST, Vincenzo Pupillo ha scritto:
> In data venerdì 7 giugno 2024 15:44:44 CEST, Eli Zaretskii ha scritto:
> > > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > > Cc: 71380 <at> debbugs.gnu.org
> > > Date: Fri, 07 Jun 2024 14:50:24 +0200
> > > 
> > > In data venerdì 7 giugno 2024 13:12:25 CEST, Eli Zaretskii ha scritto:
> > > > > > > +(defun php-ts-mode--array-element-heuristic (node parent bol
> > > > > > > &rest _)
> > > > > > > +  "Return of the position of the first element of the array.
> > > > > > 
> > > > > > The "of" part should be deleted here, I think.
> > > > > 
> > > > > I'm not sure how to explain it. Different indentation styles indent
> > > > > the
> > > > > elements of an array differently when written on multiple rows. For
> > > > > example. in PSR2 it is like this:
> > > > > $a = array("a" => 1,
> > > > > 
> > > > >      "b" => 2,
> > > > >      "c" => 3);
> > > > > 
> > > > > while with Zend it is like this:
> > > > > $a = array("a" => 1,
> > > > > 
> > > > >            "b" => 2,
> > > > >            "c" => 3);
> > > > > 
> > > > > What do you suggest?
> > > > 
> > > > What does the function return in each of these two cases?
> > > 
> > > If '$a = array(' is on the same line as '“a” => 1,' it returns the
> > > initial position of '“a”, otherwise the starting position of 'array(',
> > > like PSR2.
> > > In terms of tree-sitter-php the first case is:
> > > (treesit-node-start (treesit-node-child parent 2))
> > > while the second is: parent indentation + offset.
> > 
> > Does it return a buffer position or a column?  You seem to say that
> > sometimes it returns the former and sometimes the latter.
> 
> The point in both cases.
> 
> Vincenzo

[0001-Add-php-ts-mode.patch (text/x-patch, attachment)]

Message #59 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sat, 08 Jun 2024 13:45:08 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: 71380 <at> debbugs.gnu.org
> Date: Sat, 08 Jun 2024 11:31:05 +0200
> 
> this is the patch update. I followed Stefan's advice and removed the 
> modification of the 'mode-line' and the comment in the 'php-ts-mode--syntax-
> table'. 

Thanks, but does this replace both patches you sent in a previous
message?  See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71380#23
Or does this replace only one of the two, and the other one is still
needed?

Message #62 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sat, 08 Jun 2024 13:15:58 +0200

In data sabato 8 giugno 2024 12:45:08 CEST, Eli Zaretskii ha scritto:
> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > Cc: 71380 <at> debbugs.gnu.org
> > Date: Sat, 08 Jun 2024 11:31:05 +0200
> > 
> > this is the patch update. I followed Stefan's advice and removed the
> > modification of the 'mode-line' and the comment in the
> > 'php-ts-mode--syntax- table'.
> 
> Thanks, but does this replace both patches you sent in a previous
> message?  See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71380#23
> Or does this replace only one of the two, and the other one is still
> needed?
Replace only one. The other is still needed. I made a separate patch, waiting 
to hear if I should open another bug for html-ts-mode. 

Thanks

Vincenzo

Message #65 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sun, 09 Jun 2024 16:53:28 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: 71380 <at> debbugs.gnu.org
> Date: Fri, 07 Jun 2024 12:45:05 +0200
> 
> +  (if (treesit-ready-p 'html)
> +      (error "Tree-sitter for HTML isn't available")

Is this really correct? we signal an error if the HTML parser is
available?

Message #68 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sun, 09 Jun 2024 16:54:38 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: 71380 <at> debbugs.gnu.org
> Date: Sat, 08 Jun 2024 13:15:58 +0200
> 
> In data sabato 8 giugno 2024 12:45:08 CEST, Eli Zaretskii ha scritto:
> > > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > > Cc: 71380 <at> debbugs.gnu.org
> > > Date: Sat, 08 Jun 2024 11:31:05 +0200
> > > 
> > > this is the patch update. I followed Stefan's advice and removed the
> > > modification of the 'mode-line' and the comment in the
> > > 'php-ts-mode--syntax- table'.
> > 
> > Thanks, but does this replace both patches you sent in a previous
> > message?  See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71380#23
> > Or does this replace only one of the two, and the other one is still
> > needed?
> Replace only one. The other is still needed. I made a separate patch, waiting 
> to hear if I should open another bug for html-ts-mode. 

Would you mind posting both patches together, updated so that I could
install them both?  And note that I just found a strange mistake(?) in
the second patch.

Thanks.

Message #71 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sun, 09 Jun 2024 20:49:24 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: 71380 <at> debbugs.gnu.org
> Date: Sun, 09 Jun 2024 19:23:05 +0200
> 
> > > > Thanks, but does this replace both patches you sent in a previous
> > > > message?  See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71380#23
> > > > Or does this replace only one of the two, and the other one is still
> > > > needed?
> > > 
> > > Replace only one. The other is still needed. I made a separate patch,
> > > waiting to hear if I should open another bug for html-ts-mode.
> > 
> > Would you mind posting both patches together, updated so that I could
> > install them both?  And note that I just found a strange mistake(?) in
> > the second patch.
> > 
> > Thanks.
> Hi Eli, 
> sorry. I made a mistake with the second patch.
> The warning Andrea reported was actually due to the (require 'html-ts-mode) at 
> the beginning of php-ts-mode.
> I then use a solution similar to elixir-ts-mode, the (require 'html-ts-mode) 
> is done only after checking that the parser exists.

But the patch you posted now doesn't include the HTML part, does it?

is the patch self-contained, or does it still need the second patch?

> There is one problem I don't know how to solve: enabling php-ts-mode, because 
> of the require html-ts-mode, the major mode for html changes from mhtml-mode 
> to html-ts-mode. 
> Can you tell me how to fix this?

Just document this side effect, so that users know.

Message #74 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sun, 09 Jun 2024 19:23:05 +0200

[Message part 1 (text/plain, inline)]

In data domenica 9 giugno 2024 15:54:38 CEST, Eli Zaretskii ha scritto:
> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > Cc: 71380 <at> debbugs.gnu.org
> > Date: Sat, 08 Jun 2024 13:15:58 +0200
> > 
> > In data sabato 8 giugno 2024 12:45:08 CEST, Eli Zaretskii ha scritto:
> > > > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > > > Cc: 71380 <at> debbugs.gnu.org
> > > > Date: Sat, 08 Jun 2024 11:31:05 +0200
> > > > 
> > > > this is the patch update. I followed Stefan's advice and removed the
> > > > modification of the 'mode-line' and the comment in the
> > > > 'php-ts-mode--syntax- table'.
> > > 
> > > Thanks, but does this replace both patches you sent in a previous
> > > message?  See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71380#23
> > > Or does this replace only one of the two, and the other one is still
> > > needed?
> > 
> > Replace only one. The other is still needed. I made a separate patch,
> > waiting to hear if I should open another bug for html-ts-mode.
> 
> Would you mind posting both patches together, updated so that I could
> install them both?  And note that I just found a strange mistake(?) in
> the second patch.
> 
> Thanks.
Hi Eli, 
sorry. I made a mistake with the second patch.
The warning Andrea reported was actually due to the (require 'html-ts-mode) at 
the beginning of php-ts-mode.
I then use a solution similar to elixir-ts-mode, the (require 'html-ts-mode) 
is done only after checking that the parser exists.
There is one problem I don't know how to solve: enabling php-ts-mode, because 
of the require html-ts-mode, the major mode for html changes from mhtml-mode 
to html-ts-mode. 
Can you tell me how to fix this?

Thanks.
Vincenzo

[0001-Add-php-ts-mode.patch (text/x-patch, attachment)]

Message #77 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sun, 09 Jun 2024 21:37:40 +0200

[Message part 1 (text/plain, inline)]

In data domenica 9 giugno 2024 19:49:24 CEST, Eli Zaretskii ha scritto:
> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > Cc: 71380 <at> debbugs.gnu.org
> > Date: Sun, 09 Jun 2024 19:23:05 +0200
> > 
> > > > > Thanks, but does this replace both patches you sent in a previous
> > > > > message?  See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71380#23
> > > > > Or does this replace only one of the two, and the other one is still
> > > > > needed?
> > > > 
> > > > Replace only one. The other is still needed. I made a separate patch,
> > > > waiting to hear if I should open another bug for html-ts-mode.
> > > 
> > > Would you mind posting both patches together, updated so that I could
> > > install them both?  And note that I just found a strange mistake(?) in
> > > the second patch.
> > > 
> > > Thanks.
> > 
> > Hi Eli,
> > sorry. I made a mistake with the second patch.
> > The warning Andrea reported was actually due to the (require
> > 'html-ts-mode) at the beginning of php-ts-mode.
> > I then use a solution similar to elixir-ts-mode, the (require
> > 'html-ts-mode) is done only after checking that the parser exists.
> 
> But the patch you posted now doesn't include the HTML part, does it?
> 
> is the patch self-contained, or does it still need the second patch?

It is self-contained. You can delete the patch for html-ts-mode, it is no 
longer needed

> 
> > There is one problem I don't know how to solve: enabling php-ts-mode,
> > because of the require html-ts-mode, the major mode for html changes from
> > mhtml-mode to html-ts-mode.
> > Can you tell me how to fix this?
> 
> Just document this side effect, so that users know.

Ok done. 

Thank you.
Vincenzo

[0001-Add-php-ts-mode.patch (text/x-patch, attachment)]

Message #80 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Sun, 09 Jun 2024 22:36:35 +0200

[Message part 1 (text/plain, inline)]

Sorry. The correct patch is this one. I had forgotten to extend the commit by 
adding documentation about the side effect.
Thanks. 
Vincenzo

In data domenica 9 giugno 2024 21:37:40 CEST, Vincenzo Pupillo ha scritto:
> In data domenica 9 giugno 2024 19:49:24 CEST, Eli Zaretskii ha scritto:
> > > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > > Cc: 71380 <at> debbugs.gnu.org
> > > Date: Sun, 09 Jun 2024 19:23:05 +0200
> > > 
> > > > > > Thanks, but does this replace both patches you sent in a previous
> > > > > > message?  See
> > > > > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71380#23
> > > > > > Or does this replace only one of the two, and the other one is
> > > > > > still
> > > > > > needed?
> > > > > 
> > > > > Replace only one. The other is still needed. I made a separate
> > > > > patch,
> > > > > waiting to hear if I should open another bug for html-ts-mode.
> > > > 
> > > > Would you mind posting both patches together, updated so that I could
> > > > install them both?  And note that I just found a strange mistake(?) in
> > > > the second patch.
> > > > 
> > > > Thanks.
> > > 
> > > Hi Eli,
> > > sorry. I made a mistake with the second patch.
> > > The warning Andrea reported was actually due to the (require
> > > 'html-ts-mode) at the beginning of php-ts-mode.
> > > I then use a solution similar to elixir-ts-mode, the (require
> > > 'html-ts-mode) is done only after checking that the parser exists.
> > 
> > But the patch you posted now doesn't include the HTML part, does it?
> > 
> > is the patch self-contained, or does it still need the second patch?
> 
> It is self-contained. You can delete the patch for html-ts-mode, it is no
> longer needed
> 
> > > There is one problem I don't know how to solve: enabling php-ts-mode,
> > > because of the require html-ts-mode, the major mode for html changes
> > > from
> > > mhtml-mode to html-ts-mode.
> > > Can you tell me how to fix this?
> > 
> > Just document this side effect, so that users know.
> 
> Ok done.
> 
> Thank you.
> Vincenzo

[0001-Add-php-ts-mode.patch (text/x-patch, attachment)]

Message #83 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <vincenzo.pupillo <at> lpsd.it>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 71380 <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Wed, 12 Jun 2024 11:25:20 +0200

[Message part 1 (text/plain, inline)]

Hi Eli, this updated patch add the support for PHP var_modifier. 

Thanks.
Vincenzo


In data domenica 9 giugno 2024 22:36:35 CEST, Vincenzo Pupillo ha scritto:
> Sorry. The correct patch is this one. I had forgotten to extend the commit by 
> adding documentation about the side effect.
> Thanks. 
> Vincenzo
> 
> In data domenica 9 giugno 2024 21:37:40 CEST, Vincenzo Pupillo ha scritto:
> > In data domenica 9 giugno 2024 19:49:24 CEST, Eli Zaretskii ha scritto:
> > > > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > > > Cc: 71380 <at> debbugs.gnu.org
> > > > Date: Sun, 09 Jun 2024 19:23:05 +0200
> > > > 
> > > > > > > Thanks, but does this replace both patches you sent in a previous
> > > > > > > message?  See
> > > > > > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71380#23
> > > > > > > Or does this replace only one of the two, and the other one is
> > > > > > > still
> > > > > > > needed?
> > > > > > 
> > > > > > Replace only one. The other is still needed. I made a separate
> > > > > > patch,
> > > > > > waiting to hear if I should open another bug for html-ts-mode.
> > > > > 
> > > > > Would you mind posting both patches together, updated so that I could
> > > > > install them both?  And note that I just found a strange mistake(?) in
> > > > > the second patch.
> > > > > 
> > > > > Thanks.
> > > > 
> > > > Hi Eli,
> > > > sorry. I made a mistake with the second patch.
> > > > The warning Andrea reported was actually due to the (require
> > > > 'html-ts-mode) at the beginning of php-ts-mode.
> > > > I then use a solution similar to elixir-ts-mode, the (require
> > > > 'html-ts-mode) is done only after checking that the parser exists.
> > > 
> > > But the patch you posted now doesn't include the HTML part, does it?
> > > 
> > > is the patch self-contained, or does it still need the second patch?
> > 
> > It is self-contained. You can delete the patch for html-ts-mode, it is no
> > longer needed
> > 
> > > > There is one problem I don't know how to solve: enabling php-ts-mode,
> > > > because of the require html-ts-mode, the major mode for html changes
> > > > from
> > > > mhtml-mode to html-ts-mode.
> > > > Can you tell me how to fix this?
> > > 
> > > Just document this side effect, so that users know.
> > 
> > Ok done.
> > 
> > Thank you.
> > Vincenzo
> 
> 

[0001-Add-php-ts-mode.patch (text/x-patch, attachment)]

Message #88 received at 71380-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <vincenzo.pupillo <at> lpsd.it>
Cc: 71380-done <at> debbugs.gnu.org
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Wed, 12 Jun 2024 21:27:14 +0300

> From: Vincenzo Pupillo <vincenzo.pupillo <at> lpsd.it>
> Cc: 71380 <at> debbugs.gnu.org
> Date: Wed, 12 Jun 2024 11:25:20 +0200
> 
> Hi Eli, this updated patch add the support for PHP var_modifier. 

Thanks, installed, and closing the bug.

I wonder if you'd like to add some tests for this new mode?

Message #91 received at 71380 <at> debbugs.gnu.org (full text, mbox):

From: Vincenzo Pupillo <vincenzo.pupillo <at> lpsd.it>
To: 71380 <at> debbugs.gnu.org, eliz <at> gnu.org, v.pupillo <at> gmail.com
Subject: Re: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Wed, 12 Jun 2024 22:44:23 +0200

In data mercoledì 12 giugno 2024 20:27:14 CEST, Eli Zaretskii ha scritto:
> > From: Vincenzo Pupillo <vincenzo.pupillo <at> lpsd.it>
> > Cc: 71380 <at> debbugs.gnu.org
> > Date: Wed, 12 Jun 2024 11:25:20 +0200
> > 
> > Hi Eli, this updated patch add the support for PHP var_modifier.
> 
> Thanks, installed, and closing the bug.
> 
> I wonder if you'd like to add some tests for this new mode?
Yes, I will write some tests in the next few weeks.

I'm currently preparing a patch to color inline css exactly like with css-ts-
mode.
Thank you.

Vincenzo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.