Fix the issues reported by PHPCS

I am working on this. I will provide a patch shortly.

Thanks

Resolved all issues except

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig ckeditor5_mentions/

FILE: ...web/modules/contrib/ckeditor5_mentions/src/Element/MentionsIntegration.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 61 | WARNING | Editor::loadMultiple calls should be avoided in classes, use
    |         | dependency injection instead
--------------------------------------------------------------------------------

Time: 1.81 secs; Memory: 12MB

Please review the patch. Thanks

Reviewed ! Patch #7 looks good to me. Thanks!

Hi akshaydalvi212,

I applied your patch #7 to the CKEditor5 Mentions module version 1.1.0 in my local and I saw a PHPCS warning relating to ckeditor5_mentions.info.yml, I tried to clone the module and apply your patch, and after I ran the PHPCS command no warning was shown. hence I will move this to RTBC.

Please look at the screenshots attached as your reference.

Thank you.

+    "use strict";
+    var e = {
+        d: (t, i) => {
+            for (var o in i) { e.o(i, o) && !e.o(t, o) && Object.defineProperty(t, o, {
+                enumerable: !0,
+                get: i[o]
+            })
+        },
+        o: (e, t) => Object.prototype.hasOwnProperty.call(e, t)
+    },
+    t = {};
+            }
+    e.d(t, {
+    default: () => i
+    });
+    var active_editors = drupalSettings.ckeditor5Premium.active_editors;
+    const i = {
+        MentionsIntegration: class {
+            constructor(e) {
+                if (this.editor = e, void 0 === this.editor.plugins._availablePlugins || !this.editor.plugins._availablePlugins.has("Mention") || void 0 === drupalSettings.ckeditor5Premium || void 0 === 

The indentation is still wrong, since also JavaScript files use an indentation of two spaces ( https://www.drupal.org/docs/develop/standards/javascript/javascript-codi... → ).

    * @param \Symfony\Component\HttpFoundation\RequestStack $requestStack
    *   Request stack.
+   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
+   *   The entity type manager.
    */
   public function __construct(
     protected MentionDataProvider $mentionsProvider,
     protected MentionSettings $mentionSettings,
-    protected RequestStack $requestStack
+    protected RequestStack $requestStack,
+    EntityTypeManagerInterface $entity_type_manager
   ) {
+    $this->entityTypeManager = $entity_type_manager;

Since that code is changed, function/method declarations must be written in a single line, as per Drupal coding standards.

+        // Unused variable $minimum_characters
+        // $minimum_characters = $mention_feed->get('minimum_characters');.

Code that is not used is removed, not commented out.
Also, periods are not added to commented out code, since code is not a sentence.

I will provide an updated patch which will solve the suggested changes and interdiff file.

providing updated patch and interdiff files.
kindly review.

Hi, patch #13 applied cleanly and changes suggested in #11 has been incorporated in patch #13.
Attached screenshot for reference.
Thankyou.

Hi, applied patch provided in #13, there were few phpcs issues left after applying. I've attached a patch for the remaining issues. Please review this patch.

Let's use the issue fork, which has been already created.

Patch #17 doesn't apply. Pushed patch #15 changes in the MR.

Hi,
MR!2[#20] fixes all the errors and warnings.

sujan shrestha → made their first commit to this issue’s fork.

Hi @nitin_lama,

Your changes on the MR!2 was applied not-so successfully and there are still errors not fixed. Please see below:

ckeditor5_mentions git:(main) ✗ curl https://git.drupalcode.org/project/ckeditor5_mentions/-/merge_requests/2.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 52033    0 52033    0     0   156k      0 --:--:-- --:--:-- --:--:--  160k
patching file README.md
patching file ckeditor5_mentions.ckeditor5.yml
patching file ckeditor5_mentions.info.yml
Hunk #1 FAILED at 3.
1 out of 1 hunk FAILED -- saving rejects to file ckeditor5_mentions.info.yml.rej
patching file ckeditor5_mentions.module
patching file ckeditor5_mentions.services.yml
patching file js/build/mentionsIntegration.js
patching file js/ckeditor5_mentions.js
patching file js/ckeditor5_plugins/mentionsIntegration/src/mentionsIntegration.js
patching file src/Config/SettingsConfigHandler.php
patching file src/Controller/MentionAutocompleteController.php
patching file src/DataProvider/MentionDataProvider.php
patching file src/Element/MentionsIntegration.php
patching file src/Entity/Mention.php
patching file src/Entity/MentionFeed.php
patching file src/Form/MentionFeedForm.php
patching file src/Form/SharedBuildConfigFormBase.php
patching file src/Plugin/CKEditor5Plugin/ExportBase.php
patching file src/Plugin/CKEditor5Plugin/Mention.php
patching file src/Plugin/CKEditor5Plugin/Mentions.php
patching file src/Utility/Html.php
patching file src/Utility/HtmlHelper.php
patching file src/Utility/MentionSettings.php
patching file src/Utility/MentionsIntegrator.php
➜  ckeditor5_mentions git:(main) ✗ ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig ckeditor5_mentions

FILE: ...-orgissue/web/modules/contrib/ckeditor5_mentions/ckeditor5_mentions.module
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 8 | ERROR | [x] Expected strict_types=1, found strict_types = 1.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...ontrib/ckeditor5_mentions/src/Plugin/CKEditor5Plugin/CollaborationBase.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 31 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...es/contrib/ckeditor5_mentions/src/Plugin/CKEditor5Plugin/CloudServices.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 31 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...dules/contrib/ckeditor5_mentions/src/Plugin/CKEditor5Plugin/ExportBase.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 77 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...web/modules/contrib/ckeditor5_mentions/src/Element/MentionsIntegration.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 42 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...ontrib/ckeditor5_mentions/src/Controller/MentionAutocompleteController.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------------
 45 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
 45 | ERROR | [x] The closing parenthesis of a multi-line function declaration
    |       |     must be on a new line
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Time: 787ms; Memory: 12MB

Kindly check

Thanks,
Jake

MR !2 has been already merged.

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

Automatically closed - issue fixed for 2 weeks with no activity.

This issue introduced a services.yml entry, targeting a non-existent class:

  ckeditor5_mentions.editor_manager:
    class: Drupal\ckeditor5_mentions\EditorManager
    arguments:
      - ['@entity.manager']

(Not to mention, that there is no '@entity.manager' service)

50% of the changes here make no sense and completely blow up the module.

avpaderno → changed the visibility of the branch 3353151-gitlab-ci-reports to active.

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

Commit 3e66ab5f0888486a6ad2e417da24bd76a0bf10e2 should not have been done.

Yea all good, I'll provide fixes in https://git.drupalcode.org/issue/ckeditor5_mentions-3353151/-/tree/33531...

It is probably better to create a new issue asking to revert the changes done in that commit. Once that is done, a new issue can be created to fix the PHP_CodeSniffer warnings/errors reported by GitLab CI for the 2.0.x branch.

The 1.1.0 version is no longer supported.

avpaderno → changed the visibility of the branch 3353151-phpcs-fixes to hidden.

Ok nevermind, I just realized, that the changes here were merged into both master and 2.0.x, and they were fixed by the maintainer in 2.0.x but not in "master".

Since "master" is the default branch I got quite confused.

I guess it still makes sense to merge https://git.drupalcode.org/project/ckeditor5_mentions/-/merge_requests/4, but it is not that important anymore.

Created a follow-up issue to remove the master branch and set 2.0.x as the default branch here: 🐛 Remove master branch and make 2.0.x the default branch Active .

Setting back to "Needs Review" for the small EntityTypeManager fix.

Should this issue be closed, then?

(Cross-posting caused the status change.)

Should this issue be closed, then?

I mean there are still a few phpcs fixes to fix:

FILE: /var/www/html/web/modules/contrib/ckeditor5_mentions/src/Element/MentionsIntegration.php
----------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------
 61 | WARNING | Editor::loadMultiple calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/ckeditor5_mentions/src/Utility/HtmlHelper.php
-------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
-------------------------------------------------------------------------------------
 80 | ERROR | String concat is not required here; use a single string instead
 97 | ERROR | String concat is not required here; use a single string instead
 98 | ERROR | String concat is not required here; use a single string instead
-------------------------------------------------------------------------------------

But yea, I guess it can be closed again.

@grevil If those PHP_CodeSniffer are for the 2.0.x branch, this issue can be kept open.