Problem/Motivation

In #3212521: [backport] cspell dislikes identifer in core/modules/views/src/Plugin/views/filter/FilterPluginBase.php and will fail any patch touching that file we discovered the cspell dictionaries were changed in #3211810: [security] Update Nightwatch and locked dev dependencies to address security issues and were causing problems for one specific file.

Seeing that it's highly unlikely there's only one affected files, I've ran cspell on all the files.
Note: Normally only the changed files in a patch.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#19 3212547-19.patch5.6 KBquietone

Issue fork drupal-3212547

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Spokje created an issue. See original summary.

spokje’s picture

I've ran cd core && yarn install && yarn run spellcheck -c .cspell.json "/path/to/drupal/**/*" on 9.3.x.

After deleting a lot of /vendor warnings (I know there must be a command to exclude directories, too lazy to google more than once ;), the results aren't as bad as expected":

yarn run v1.22.10
$ cspell -c .cspell.json '/path/to/drupal/**/*'

yarn run v1.22.10
$ cspell -c .cspell.json '/path/to/drupal/**/*'

core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:18 - Unknown word (consectetur)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:30 - Unknown word (adipisicing)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:55 - Unknown word (eiusmod)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:63 - Unknown word (tempor)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:70 - Unknown word (incididunt)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:84 - Unknown word (labore)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:107 - Unknown word (aliqua)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:140 - Unknown word (quis)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:145 - Unknown word (nostrud)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:178 - Unknown word (ullamco)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:190 - Unknown word (laboris)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:206 - Unknown word (aliquip)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:220 - Unknown word (commodo)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:228 - Unknown word (consequat)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:239 - Unknown word (Duis)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:244 - Unknown word (aute)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:249 - Unknown word (irure)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:264 - Unknown word (reprehenderit)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:281 - Unknown word (voluptate)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:291 - Unknown word (velit)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:319 - Unknown word (fugiat)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:326 - Unknown word (nulla)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:332 - Unknown word (pariatur)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:342 - Unknown word (Excepteur)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:352 - Unknown word (sint)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:357 - Unknown word (occaecat)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:366 - Unknown word (cupidatat)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:380 - Unknown word (proident)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:390 - Unknown word (sunt)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:408 - Unknown word (officia)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:416 - Unknown word (deserunt)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:425 - Unknown word (mollit)
core/modules/color/tests/modules/color_test/themes/aa/color/preview.html:4:444 - Unknown word (laborum)
core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php:268:19 - Unknown word (iteratable)

Still needs fixing though, opening a MR.

spokje’s picture

Although it looks like adding a /* cspell:disable */ to core/modules/color/preview.html passes all color module related tests (See: https://git.drupalcode.org/project/drupal/-/blob/475bc9f6339d1a7194368b455d7ee5108815acdb/core/modules/color/preview.html and https://www.drupal.org/pift-ci-job/2053211), I'm highly doubtful this is what we want.

Firing a new test with only changes in core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php and let reviewers/committers decide what they wanna do.

spokje’s picture

Assigned: spokje » Unassigned
Status: Needs work » Needs review

Since 9.2.x has the same unknown word list as 9.3.x, the same patch could be applied over there.

No clue about the status of 9.1.x which is freezed according to @catch #3212521-12: [backport] cspell dislikes identifer in core/modules/views/src/Plugin/views/filter/FilterPluginBase.php and will fail any patch touching that file, so I'll just leave that alone.

Which MR to use is at reviewers/committers discression:

MR641: 2 changes, not too sure about core/modules/color/preview.html
MR642: 1 change

alexpott’s picture

It'd be cool if core/scripts/dev/commit-code-check.sh could detect a change in the dictionary and re-run the tests. We can test if the dictionary is in $FILES - also we should only do this on DrupalCI - there's a flag for that too. Also we could extend this to:

  • running PHPCS on all files if phpcs.xml.dist is updated
  • running all JS build checks if core/yarn.lock is updated
spokje’s picture

longwave’s picture

We already have this line in .cspell.json:

      "modules/color/preview.html",

Maybe we need to change that to

      "**/color/preview.html",

spokje’s picture

Nice catch @longwave!

That indeed does the trick, created new MR with that addition and sunk the two existing MRs to prevent confusion.

alexpott’s picture

The command to correctly run spellcheck on core is cd core && yarn install && yarn run spellcheck:core

alexpott’s picture

Status: Needs review » Needs work
spokje’s picture

Status: Needs work » Needs review
quietone’s picture

Status: Needs review » Reviewed & tested by the community

I ran spellcheck on the code base before applying this patch and there was one error, iteratable. I applied the diff and ran commit-code-check and no errors reported.

This is good to go.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

There's another side of this we should re-run the command to make our custom dictionary because maybe more words have been added upstream... so we need to run yarn run spellcheck:make-drupal-dict - note it is important that this is done on a clean checkout with no changes to the root composer.json

quietone’s picture

Status: Needs work » Needs review
StatusFileSize
new5.6 KB

@alexpott, thanks.

New patch created after running yarn run spellcheck:make-drupal-dict . Among the changes to the dictionary it adds 'iteratable' so this does not include the fix for that word in SourcePluginBase. It seems best to handle that in #3122088: [Meta] Remove spelling errors from dictionary.txt and fix them. Hope that is right.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

#3210633: Update JavaScript dependencies for Drupal 9.2 is also trying to fix the dictionaries, maybe we should just get this in here and then we don't need to worry about it over there.

I get a clean run of yarn spellcheck:core with #19 applied so this is RTBC.

  • catch committed 50b8941 on 9.3.x
    Issue #3212547 by Spokje, quietone, alexpott, longwave: cspell...

  • catch committed b8a8063 on 9.2.x
    Issue #3212547 by Spokje, quietone, alexpott, longwave: cspell...
catch’s picture

Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

Status: Fixed » Closed (fixed)

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