all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@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>
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 09:58:11 +0300	[thread overview]
Message-ID: <86cyouimm4.fsf@gnu.org> (raw)
In-Reply-To: <3686989.dWV9SEqChM@3-191.divsi.unimi.it> (message from Vincenzo Pupillo on Wed, 05 Jun 2024 15:59:20 +0200)

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





  reply	other threads:[~2024-06-06  6:58 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 [this message]
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
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=86cyouimm4.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=71380@debbugs.gnu.org \
    --cc=casouri@gmail.com \
    --cc=monnier@iro.umontreal.ca \
    --cc=philipk@posteo.net \
    --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 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.