Fix the issues reported by phpcs

+/**
+ * @file
+ * Contains editor_mailto_link.module.
+ */

That is not much helpful, especially because it says the file contains itself.

+/**
+ * Function to parse url field.
+ */

It does not describe the parameters nor the return value.

+/**
+ * @file
+ * Primary module hooks for editor mailto link.
+ */
+

There is no need to say they are module hooks. Probably, it would be better to say Hook implementations for [module name]. The module name should use upper-case characters where they are used in the module name. (See the project name, which is shown on the breadcrumb of this page.)

+/**
+ * Function to parse url field.
+ *
+ * @param string $href
+ *   The url href.
+ *
+ * @return array
+ *   The array of mailto link.

url is not correctly spelled. The last line still does not make clear what the return value is. What is an array of mailto link?

+/**
+ * Validation handler for editor_mailto_link_form_editor_link_dialog_alter().
+ */

What that line should say is the is the class implementing the form, not the function that adds the validation handler.

+/**
+ * Function to build query parameter.
+ */
 function _editor_mailto_link_build_mailto_link(array $mailto_values) {

Parameters and return value are not documented. Also, it is useless to start a document comment for a function with Function to, which is not even what the Drupal coding standards says to use for those comments.

Working on this.

Updated the patch.
Please review.

+ * @return array
+ *   Returns the array of links.

Returns is not used in the line describing the return value.

+/**
+ * The validation handler.
+ *
+ * @param array $form
+ *   The form.
+ * @param Drupal\Core\Form\FormStateInterface $form_state
+ *   The form state.
+ */

The short description still does not say for which that function is a validation handler. Also, the parameters are not described for validation and submission handlers. API documentation and comment standards / Form-generating functions → has example code, although it still Drupal 7 code.

/**
 * Form validation handler for user_login_form().
 *
 * @see user_login_form_submit()
 */
function user_login_form_validate($form, &$form_state) {
  // …
}

/**
 * Form submission handler for user_login_form().
 *
 * @see user_login_form_validate()
 */
function user_login_form_submit($form, &$form_state) {
  // …
}

Working on this.

Updated patch.
Please review.

Patch #29 seems to have incorporated the raised changes by @apaderno in #27 except the 1st one.

Working on this.

Patch updated.
Please review.

Hi @Rhaki,

Confirmed fixed the PHPCS errors after applying patch #32 to the Editor Mailto Link against 1.0.x-dev and with Drupal core 9.5.6.

Moving this now to RTBC.
Kindly refer to the attached screenshots, please.

Thank you.

+ * @param array $mailto_values
+ *   The array of mailto form values.

The description is not much clear and it repeats the parameter type, which is already given from the @param line.

+ * @return mixed
+ *   Returns the query parameters.

The return value description must not start with Returns.

Fixed the issues mentioned in #34

Hi @sourabhjain,

Confirmed no PHPCS errors shown after applying patch #35 to the Editor Mailto Link module 1.0.x-dev and with Drupal core 9.5.8.

Moving this now to RTBC.
Kindly refer to the attached screenshots, please.

+ * The URL field parser.
+ *
+ * @param string $href
+ *   The URL href.
+ *
+ * @return array
+ *   The type and href.
+ */
+function _editor_mailto_link_parse_url_field(string $href) {

The short description does not describe what the function does.
The return value description does not correctly describes what the returned array contains.

+ * @param array $mailto_values
+ *   The mailto form values i.e. email, subject, body.

It is preferable not to use i.e.

Please review.

+/**
+ * @file
+ * Hook implementations for Editor Mailto Link.
+ */
+

It is Hook implementations for the Editor Mailto Link module.

+/**
+ * The URL field parser based on user values.
+ *

The description does not say much about the function. bases on user values is not necessary, as most of the functions handle user input.

+ * @return array
+ *   The type of field and it's content.

It would be its content.
That is not sufficient to describe what the function returns, since the function returns an array with different array indexes basing on the link type.

+/**
+ * Builds query parameters.
+ *
+ * @param array $mailto_values
+ *   The mailto form values such as email, subject, body.
+ *
+ * @return mixed
+ *   The query parameters.
+ */
 function _editor_mailto_link_build_mailto_link(array $mailto_values) {

Truly, it build the mailto: link.
$mailto_values is an array of values used to build the mailto: link. Whenever they come from a form is irrelevant for the function.
The return value is the mailto: link, not query parameters. It is a string, not mixed values.

Please review.

Hi @sourabhjain,

Confirmed no PHPCS errors were shown after applying patch #40.

Checking patch editor_mailto_link.info.yml...
Checking patch editor_mailto_link.module...
Applied patch editor_mailto_link.info.yml cleanly.
Applied patch editor_mailto_link.module cleanly.

Please see the attached file for reference.

Moving this now to RTBC.
Thank you!

The patch could apply, but that does not mean the changes are correct.

+/**
+ * The URL field parser whether the field is mail or telephone.

Grammatically, that does not make sense. As sentence, it has a subject (The URL field parser), but it doesn't have a verb. (whether is not a verb, but an adverb.)

+/**
+ * Builds query parameters.
+ *
+ * @param array $mailto_values
+ *   The mailto values such as email, subject, body.
+ *
+ * @return string
+ *   The mailto: link.
+ */
 function _editor_mailto_link_build_mailto_link(array $mailto_values) {

That is not the correct description, for a function that returns a link. (Query parameters and link are two different things.)

+ * @param array $mailto_values
+ *   The mailto values such as email, subject, body.

Probably An array containing email address, subject, and body. is a better description.

Patch updated and interdiff attached.
Please review.

Hi,

The Patch #44 was applied cleanly and no PHPCS errors were shown.

Checking patch editor_mailto_link.info.yml...
Checking patch editor_mailto_link.module...
Applied patch editor_mailto_link.info.yml cleanly.
Applied patch editor_mailto_link.module cleanly.

Please see the attached file for reference.

Moving this to RTBC.

Thank you!

+/**
+ * The URL field parser checks whether the field is mail or telephone.
+ *
+ * @param string $href
+ *   The URL href.
+ *
+ * @return array
+ *   The type of field mail or telephone.
+ */
+function _editor_mailto_link_parse_url_field(string $href) {

The URL field parser ins the description is not necessary.
That description is not correct, since the function returns the following value.

    return [
      'type' => 'mailto',
      'mailto' => [
        'email' => $matches[1],
        'body' => $output['body'] ?? NULL,
        'subject' => $output['subject'] ?? NULL,
      ],
    ];

It cannot be said to check whether the field is mail or telephone because, in that case, it would return a Boolean value.

The return value description is wrong too. It should also describe each of the array indexes the returned array contains.

+/**
+ * Form validation handler for form_editor_link_dialog_alter().
+ */

A form ID is either a string or a function name that does not end with alter(). That is probably an alter hook.

+/**
+ * Builds query parameters.
+ *
+ * @param array $mailto_values
+ *   An array containing email address, subject, and body.
+ *
+ * @return string
+ *   The query parameter.
+ */
 function _editor_mailto_link_build_mailto_link(array $mailto_values) {

The function build a query string, not query parameters.

Hi,

Patch #47 was applied cleanly and no PHPCS errors were detected.

Please see the attached file.

Moving this to RTBC.

Thank you!

+/**
+ * The URL field parser for creating array of required fields for mail.
+ *
+ * @param string $href
+ *   The URL href.
+ *
+ * @return array
+ *   Array of values with keys type and mailto.
+ */
+function _editor_mailto_link_parse_url_field(string $href) {

The description is still not correct. It is for a function, which cannot be described as The URL field parser. That is a description that could be used for a class or a class property, not for a function or a method.

+ * @return array
+ *   Array of values with keys type and mailto.

The description is missing an indefinite article and an definite article.
keys should be placed after type and mailto.
I am not sure that description is correct, as the array contains more than just two keys.

+/**
+ * Function for custom submit handler.
+ */
 function _editor_mailto_link_submit_handler(array &$form, FormStateInterface $form_state) {

If that is a form submission handler, the description is not correct.
Also, custom submit handler is vague; it is not necessary to say it is custom, which then could be said for any form submission handler added by modules that do not implement the form.

+/**
+ * Builds query string.
+ *

The description is missing an indefinite article.

+/**
+ * The function for url field.
+ */
 function _editor_mailto_link_parse_url_field(?string $href) {

+/**
+ * The function for mailto_link.
+ */
 function _editor_mailto_link_build_mailto_link(array $mailto_values) {

The descriptions do not say what the functions do. The documentation comments do not describe the parameters nor the return values.

+/**
+ * The function for link submit.
+ */
 function _editor_mailto_link_submit_handler(array &$form, FormStateInterface $form_state) {

If that is a form submission handler, the description must be different.

Ran phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml again, found that all issues are fixed and the feedback in #51 about the description is also already addressed.

Also, if there is already an issue fork, please do not create patches. It avoids to start again from zero.

Addressed the comment in MR. Please review.

silvi.addweb → made their first commit to this issue’s fork.

Hi,

I have applied the latest MR !2 to the Editor Mailto Link module with 1.0.x-dev on Drupal 10 and confirmed all PHPCS errors have been fixed.

I ran this command on the module:
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml editor_mailto_link

Please see the attached file for reference.

I'm moving this now to ‘RTBC’.

Thank you!

avpaderno → changed the visibility of the branch 3291030-drupalcoding-standard-issue to hidden.

avpaderno → changed the visibility of the branch 3291030-gitlab-ci-reports to hidden.