From mboxrd@z Thu Jan  1 00:00:00 1970
Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail
From: Eli Zaretskii <eliz@gnu.org>
Newsgroups: gmane.emacs.bugs
Subject: bug#71380: 30.0.50; Submitting php-ts-mode, new major mode for php
Date: Thu, 06 Jun 2024 09:58:11 +0300
Message-ID: <86cyouimm4.fsf@gnu.org>
References: <3686989.dWV9SEqChM@3-191.divsi.unimi.it>
Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214";
	logging-data="24072"; mail-complaints-to="usenet@ciao.gmane.io"
Cc: 71380@debbugs.gnu.org
To: Vincenzo Pupillo <v.pupillo@gmail.com>,
 Stefan Monnier <monnier@iro.umontreal.ca>,
 Philip Kaludercic <philipk@posteo.net>, Yuan Fu <casouri@gmail.com>
Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Jun 06 08:58:58 2024
Return-path: <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org>
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 <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org>)
	id 1sF75O-00064C-25
	for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 06 Jun 2024 08:58:58 +0200
Original-Received: from localhost ([::1] helo=lists1p.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.90_1)
	(envelope-from <bug-gnu-emacs-bounces@gnu.org>)
	id 1sF75H-0002JP-2D; Thu, 06 Jun 2024 02:58:51 -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 <Debian-debbugs@debbugs.gnu.org>)
 id 1sF75E-0002JE-RL
 for bug-gnu-emacs@gnu.org; Thu, 06 Jun 2024 02:58:49 -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 <Debian-debbugs@debbugs.gnu.org>)
 id 1sF75E-0000Y1-0Y
 for bug-gnu-emacs@gnu.org; Thu, 06 Jun 2024 02:58:48 -0400
Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2)
 (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1sF75S-0002df-5p
 for bug-gnu-emacs@gnu.org; Thu, 06 Jun 2024 02:59:02 -0400
X-Loop: help-debbugs@gnu.org
Resent-From: Eli Zaretskii <eliz@gnu.org>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@gnu.org
Resent-Date: Thu, 06 Jun 2024 06:59:02 +0000
Resent-Message-ID: <handler.71380.B71380.171765711810083@debbugs.gnu.org>
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.171765711810083
 (code B ref 71380); Thu, 06 Jun 2024 06:59:02 +0000
Original-Received: (at 71380) by debbugs.gnu.org; 6 Jun 2024 06:58:38 +0000
Original-Received: from localhost ([127.0.0.1]:43742 helo=debbugs.gnu.org)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <debbugs-submit-bounces@debbugs.gnu.org>)
 id 1sF754-0002cY-52
 for submit@debbugs.gnu.org; Thu, 06 Jun 2024 02:58:38 -0400
Original-Received: from eggs.gnu.org ([209.51.188.92]:50426)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@gnu.org>) id 1sF752-0002bh-8q
 for 71380@debbugs.gnu.org; Thu, 06 Jun 2024 02:58:36 -0400
Original-Received: from fencepost.gnu.org ([2001:470:142:3::e])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@gnu.org>)
 id 1sF74h-0000WI-QB; Thu, 06 Jun 2024 02:58:15 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date:
 mime-version; bh=FVXjAvd9nrpo81p9yrhc2lyyM/0t2cOmelYeOVPS3Pg=; b=CecFcHrKziF6
 7fVihnByjn/Qrg51QAgCmZ0LWGjNbAlc2aWUqdTv9jiNTW0ZTug8hQXFiIRdBkQbsgpHQHNv7Ghvi
 el5Aw6f99XzcwQxR/db1DQO2qq3dbAHHGK9nw2IRvQwqRcjbQRlmaaivQr7re9t3QexuLjNURllrG
 UXTvVJE8vkoD8tUmnX7ip1Dx1oh1vzIc8k68wyrxlQCDnI8Aog+TWSL2CEyOzdTGDbKMn57ztNvwR
 ImGnizoSIjGaW9nCetjkQmBqfVPnYayJRIHnmmMRdtalOXJ5bUYmsG7Sd5vwG2aD8IwMaYYVIp7WA
 Fd+SNzPvB5V4XLPLOgQG5A==;
In-Reply-To: <3686989.dWV9SEqChM@3-191.divsi.unimi.it> (message from Vincenzo
 Pupillo on Wed, 05 Jun 2024 15:59:20 +0200)
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" <bug-gnu-emacs.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/bug-gnu-emacs>,
 <mailto:bug-gnu-emacs-request@gnu.org?subject=unsubscribe>
List-Archive: <https://lists.gnu.org/archive/html/bug-gnu-emacs>
List-Post: <mailto:bug-gnu-emacs@gnu.org>
List-Help: <mailto:bug-gnu-emacs-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/bug-gnu-emacs>,
 <mailto:bug-gnu-emacs-request@gnu.org?subject=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:286666
Archived-At: <http://permalink.gmane.org/gmane.emacs.bugs/286666>

> From: Vincenzo Pupillo <v.pupillo@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.