Fix or ignore words that start with "v", excluding real non-English words

1. +++ b/core/lib/Drupal/Core/Composer/Composer.php
   @@ -8,6 +8,8 @@
   +// cspell:ignore vendored
   

   This is used once in a comment. I think we should reword the comment. Instead of * Remove possibly problematic test files from vendored projects. it can be * Remove possibly problematic test files from vendor projects.

2. +++ b/core/modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php
   @@ -11,6 +11,8 @@
   +// cspell:ignore vxezb
   

   This is a few characters is a placeholder and used 6 times. I wonder if it would be better to have 6 ignore lines instead.

3. +++ b/core/modules/filter/src/Plugin/Filter/FilterHtml.php
   @@ -8,6 +8,8 @@
   +// cspell:ignore vxfbk
   

   As above

4. +++ b/core/modules/quickedit/tests/src/Functional/QuickEditEndPointAccessTest.php
   @@ -7,6 +7,8 @@
   +// cspell:ignore vmov
   

   As above

5. +++ b/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditLoadingTest.php
   @@ -14,6 +14,8 @@
   +// cspell:ignore vivamus
   

   This is part of alt text for an image. Can we change it to English words and still have a valid test?

6. +++ b/core/modules/quickedit/tests/src/FunctionalJavascript/QuickEditLoadingTest.php
   @@ -14,6 +14,8 @@
   diff --git a/core/modules/search/tests/src/Kernel/SearchMatchTest.php b/core/modules/search/tests/src/Kernel/SearchMatchTest.php
   
   +++ b/core/modules/search/tests/src/Kernel/SearchMatchTest.php
   @@ -7,6 +7,8 @@
   diff --git a/core/modules/search/tests/src/Kernel/SearchSimplifyTest.php b/core/modules/search/tests/src/Kernel/SearchSimplifyTest.php
   

   The search tests are using lots of non English words. Maybe it would be better to do the search module tests in a separate issue.

I'll work on a reroll and apply the comments from @quietone #6

Finished reroll. Regarding comment #6:

1. Done.

2-4. Added // cSpell:disable-next-line for each occurrence.

5. Was solved on some other commit in between versions.

6. I left SearchMatchTest and SearchSimplifyTest unchanged so them can be addressed on a separate issue.

I haven't noticed it didn't apply.

Adding tag for the cspell spelling error issues.

Added reroll of patch #15 on Drupal 10.1.x.

Patch failed. You can test a patch before uploading with core/scripts/dev/commit-code-check.sh

Fixed patch error.

Also removed change to generate-d7 script as I wasn't sure how that pretained to this.

I found the word "vampirize" on the dictionary, and is used only once on core/lib/Drupal/Core/Render/theme.api.php: 891.
Should we '// cspell:ignore vampirize' ?

I added '// cSpell:disable-next-line' for vxfbk on core/modules/ckeditor5/src/HTMLRestrictions.php:562
and changed veeeery to very_very on core/modules/pgsql/tests/src/Unit/SchemaTest.php:77

As #6 6. "Maybe it would be better to do the search module tests in a separate issue."
I added back verygreatdrupalmodule to the dictionary found on core/modules/search/tests/src/Kernel/SearchTextProcessorTest.php:82
Or should we '// cspell:ignore verygreatdrupalmodule' too?

I am sorry, I have no idea how to fix this error....

changed the veeeery back, and added a '// cSpell:disable-next-line' on core/modules/pgsql/tests/src/Unit/SchemaTest.php:77
That fixed the error.
also added back to the dictionary the other 'veniam' and 'vocabs', with 'verygreatdrupalmodule' to became the ToDo list for "Maybe it would be better to do the search module tests in a separate issue." as comment on #6

Hi I revised the #21 patch.

Patch was applied cleanly

I also ran the spellcheck command to see if the words were ignored or rewrite with successful.

They were rewrited and ignored as expected.

Moving to RTBC.

Thanks folks for working on this.

1. +++ b/core/modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php
   @@ -280,28 +280,35 @@ public static function cases(ContainerInterface $container = NULL, AccountInterf
   +      // cSpell:disable-next-line
          '<drupal-render-placeholder callback="\Drupal\big_pipe_test\BigPipeTestController::responseException" arguments="" token="PxOHfS_QL-T01NjBgu7Z7I04tIwMp6La5vM-mVxezbU"></drupal-render-placeholder>',
          [
            '#lazy_builder' => ['\Drupal\big_pipe_test\BigPipeTestController::responseException', []],
          ]
        );
   +    // cSpell:disable-next-line
        $embedded_response_exception->bigPipePlaceholderId = 'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&amp;&amp;token=PxOHfS_QL-T01NjBgu7Z7I04tIwMp6La5vM-mVxezbU';
   +    // cSpell:disable-next-line
        $embedded_response_exception->bigPipePlaceholderRenderArray = [
   +      // cSpell:disable-next-line
          '#markup' => '<span data-big-pipe-placeholder-id="callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&amp;&amp;token=PxOHfS_QL-T01NjBgu7Z7I04tIwMp6La5vM-mVxezbU"></span>',
          '#cache' => $cacheability_depends_on_session_and_nojs_cookie,
          '#attached' => [
            'library' => ['big_pipe/big_pipe'],
            'drupalSettings' => [
              'bigPipePlaceholderIds' => [
   +            // cSpell:disable-next-line
                'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&&token=PxOHfS_QL-T01NjBgu7Z7I04tIwMp6La5vM-mVxezbU' => TRUE,
              ],
            ],
            'big_pipe_placeholders' => [
   +          // cSpell:disable-next-line
              'callback=%5CDrupal%5Cbig_pipe_test%5CBigPipeTestController%3A%3AresponseException&amp;&amp;token=PxOHfS_QL-T01NjBgu7Z7I04tIwMp6La5vM-mVxezbU' => $embedded_response_exception->placeholderRenderArray,
   

   It looks like these are all the same token. Instead of ignoring it on six separate lines, maybe we could store it to a variable? Or, would CSPell understand to ignore the whole token as one word for this file (despite its mixed case)?

   That V is not the first capital letter in this token, either, so can we check the dictionary for other entries that this fix makes unnecessary?

2. +++ b/core/modules/config_translation/tests/src/Functional/ConfigTranslationUiTest.php
   @@ -13,6 +13,8 @@
   +// cspell:ignore viewsviewfiles
   +
    /**
   

   I looked up the test, and this seems like a weird class name component to generate. Why isn't it hyphenated? Moreover, aren't all the views always going to be views.view.something, so why even add the viewview prefix at all?

   This change is fine for the scope of the cspell issue, but maybe worth a followup for Views' behavior?

3. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserProfileFieldInstanceTranslationTest.php
   @@ -4,6 +4,8 @@
   +// cspell:ignore vous
   +
   

   There are two French sentences in this file, full of French words that will fail spelling in English. Instead of just ignoring "vous", let's ignore the two French lines, and then we can remove the other corresponding words from the dictionary.

4. +++ b/core/modules/config_translation/tests/src/Kernel/Plugin/migrate/source/d6/ProfileFieldTranslationTest.php
   @@ -4,6 +4,8 @@
   +// cspell:ignore vous
   +
   

   As above.

5. +++ b/core/modules/pgsql/tests/src/Unit/SchemaTest.php
   @@ -74,6 +74,7 @@ public function providerComputedConstraintName() {
          ['user_field_data', 'a_veeeery_veery_very_super_long_field_name__key', 'drupal_BGGYAXgbqlAF1rMOyFTdZGj9zIMXZtSvEjMAKZ9wGIk_key'],
   

   I was going to say we could just replace "veeeery" and "veery" with "very", but there is also the token on the line. We still could use "very" spelled properly for consistency, though, even though the line would still be ignored for the token. We'd need to change the test to prevent the earlier fail as well.

6. +++ b/core/modules/taxonomy/tests/src/Kernel/Migrate/d7/MigrateTaxonomyTermTest.php
   @@ -6,6 +6,8 @@
   +// cspell:ignore vous
   +
   

   As above.

7. +++ b/core/modules/views/tests/src/Functional/Plugin/PagerTest.php
   @@ -7,6 +7,8 @@
   +// cspell:ignore volgende vorige
   

   These are Afrikaans words in this file. There are two other Afrikaans words also in the file that could be ignored and removed from the dictionary.

Since my suggestions for the French in the migrate tests and the Afrikaans in PagerTest are scope-creep-y, they could be split into their own issues. Overall, real words in foreign languages should be handled on a per-sentence or per-test basis, not on a per-word basis.

Thanks everyone!

I agree with most of these changes, so will work on it.

About the 5. tho, changing the word veeeery to very_very on core/modules/pgsql/tests/src/Unit/SchemaTest.php:77 makes the test to break as we can see on #18. Any suggestion on how to fix it?

I was checking and on comment #6

+++ b/core/modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php
@@ -11,6 +11,8 @@
+// cspell:ignore vxezb

This is a few characters is a placeholder and used 6 times. I wonder if it would be better to have 6 ignore lines instead.

@xjm should we keep it or change back?

and about 6. from #24 I believe is the same point made in #6 6.

For point 5, the failing test you encountered above actually tells you what to do. From https://www.drupal.org/pift-ci-job/2549835:

-'SELECT 1 FROM pg_constraint WHERE conname = 'drupal_BGGYAXgbqlAF1rMOyFTdZGj9zIMXZtSvEjMAKZ9wGIk_key''
+'SELECT 1 FROM pg_constraint WHERE conname = 'drupal_WW_a8TlbZ3UQi20UTtRlJFaIeSa6FEtQS5h4NRA3UeU_key''

You can research what this means and why it was introduced by using git log -L for the line containing the expected value:

[ayrton:drupal | Tue 12:25:56] $ git log -L77,78:core/modules/pgsql/tests/src/Unit/SchemaTest.php
commit a17303166447173af60d8d4b3fbfee37f098a692
Author: Alex Pott <alex.a.pott@googlemail.com>
Date:   Thu Sep 3 12:59:34 2015 +0100

    Issue #2561129 by plach, alexpott, catch: Composite indexes are not correctly deleted/re-created when updating a field storage definition

diff --git a/core/tests/Drupal/Tests/Core/Database/Driver/pgsql/PostgresqlSchemaTest.php b/core/tests/Drupal/Tests/Core/Database/Driver/pgsql/PostgresqlSchemaTest.php
--- /dev/null
+++ b/core/tests/Drupal/Tests/Core/Database/Driver/pgsql/PostgresqlSchemaTest.php
@@ -0,0 +77,2 @@
+      ['user_field_data', 'a_veeeery_veery_very_super_long_field_name__key', 'drupal_BGGYAXgbqlAF1rMOyFTdZGj9zIMXZtSvEjMAKZ9wGIk_key'],

That leads us to #2561129: Composite indexes are not correctly deleted/re-created when updating a field storage definition. Reading that issue, and the bug it is addressing, leads us to:
Drupal\pgsql\Driver\Database\pgsql\Schema::ensureIdentifiersLength()

This is the relevant bit of code in that method:

  $identifierName = implode($separator, [
    $table_identifier_part,
    $column_identifier_part,
    $tag,
 ]);

And:

  if (strlen($identifierName) > $this->maxIdentifierLength) {
    $saveIdentifier = '"drupal_' . $this
      ->hashBase64($identifierName) . '_' . $tag . '"';
  }

The code for the hashBase64() method is:

  protected function hashBase64($data) {
    $hash = base64_encode(hash('sha256', $data, TRUE));
    // Modify the hash so it's safe to use in PostgreSQL identifiers.           
    return strtr($hash, ['+' => '_', '/' => '_', '=' => '']);
  }

So, basically, we have to (a) make sure the key is over 63 characters for the test to work (edit: so we might need to add an extra "very" or two in there), (b) concatenate it with the table name with a __ separator, and then (c) run it through
strtr(base64_encode(hash('sha256', $data, TRUE), ['+' => '_', '/' => '_', '=' => ''])

And that should give us the new test value. :)

You can sub the value in here to test:
https://3v4l.org/s2mNI

@lucienchalom, sorry, we are crossposting. Regarding #6, I am not suggesting changing it back to what it was before, but rather doing:

// cSpell:disable-next-line
$token = 'PxOHfS_QL-T01NjBgu7Z7I04tIwMp6La5vM-mVxezbU';

...and then concatenating that variable into the six assertion lines afterward, so only the one line will need to be ignored since the rest will instead have the English word token. @quietone and I are trying to solve the same problem, I think, just differently. :) And it will clean up more than one "word" from the dictionary that way.

1. I could create the token, that was a great idea.

2. Also thank you so much for the step by step on how to fix the test of the very_very.

3. About the French words, I suggest that we create an issue for cspell on all the French words and remove them form this scope.
For the other words they are actually Dutch.

/core/modules/views/tests/src/Functional/Plugin/PagerTest.php
// Add Dutch language programmatically.
    ConfigurableLanguage::createFromLangcode($langcode)->save();

    $edit = [
      'translation[config_names][views.view.content][display][default][display_options][pager][options][tags][first]' => '« Eerste',
      'translation[config_names][views.view.content][display][default][display_options][pager][options][tags][previous]' => '‹ Vorige',
      'translation[config_names][views.view.content][display][default][display_options][pager][options][tags][next]' => 'Volgende ›',
      'translation[config_names][views.view.content][display][default][display_options][pager][options][tags][last]' => 'Laatste »',
    ];
    $this->drupalGet('admin/structure/views/view/content/translate/nl/edit');
    $this->submitForm($edit, 'Save translation');

So I think is better to leave with cspell ignore for the 4 words.

4. The last thing to do then is the viewsview.
Should we create a new issue for it?

Eh well at least Dutch and Afrikaans have a lot of cognates, right? 😅

The decision to address non-English words in a separate issue seems like a sound one to me. Can you create another child issue of the meta for that? The scope might be something like "Handle valid words from other languages consistently in cspell" or something.

Oh, and yes, I think a separate issue is good for the viewsview thing also.

A cording to Wikipedia about Afrikaans "An estimated 90 to 95% of the vocabulary is of Dutch origin with adopted words from other languages including German and the Khoisan languages of Southern Africa. There is a large degree of mutual intelligibility between the two languages, especially in written form." 😅😅😅

Anyway, here is the new issue #3333267: Handle valid words from other languages consistently in cspell. XD

And the viewsview issue #3333327: viewsviewfiles in configTranslationUiTest.

👍

I tested patch #29:

yarn run v1.22.19
$ cspell "**/*" ".*" "../composer/**/*" "../composer.json"
CSpell: Files checked: 14220, Issues found: 0 in 0 files

The ignored CSpell words in the test files work as expected.

I checked all the words we are removing from the dictionary, and it makes sense.

The lines excluded from CSpell with cSpell:disable-next-line are tokens or random strings, so I agree with the approach.

I reviewed each word change and grepped core for each as well. The changes in the patch look correct.

However, I disagree with keeping a French word in the dictionary but moving two Dutch words to an ignore line. These should be handled consistently. Plus, as pointed out in #29 there are other Dutch words in the dictionary.

I am setting back to needs works to move those words back to the dictionary.

@quietone, I already signed off on handling the foreign-language words in the dictionary (all of them) as a separate language scope.

Setting back to RTBC, but marking for release manager review while we compare notes.

However, I disagree with keeping a French word in the dictionary but moving two Dutch words to an ignore line. These should be handled consistently. Plus, as pointed out in #29 there are other Dutch words in the dictionary.

Read this a second time; actually I do agree with that. Let's update the patch. :)

Thank you for catching that, makes totally sense.

Added back volgende and vorige to the dictionary.

Confirmed issues from #36 has been addressed.

Reviewed this one more time with:
git diff --color-words='\w+|.' --staged

1. +++ b/core/lib/Drupal/Component/Transliteration/PhpTransliteration.php
   @@ -2,6 +2,8 @@
   +// cspell:ignore vibber
   +
   

   Normally, I would give the feedback that this should be above the line that contained it. However, this is the code's author's name and is in the class docblock, so it would have to be here.

2. +++ b/core/modules/config_translation/tests/src/Functional/ConfigTranslationUiTest.php
   @@ -13,6 +13,8 @@
   +// cspell:ignore viewsviewfiles
   +
   

   This is another one where I would have moved it down to the relevant line(s), but we have a followup, so this is fine for now.

3. +++ b/core/modules/pgsql/tests/src/Unit/SchemaTest.php
   @@ -74,7 +74,7 @@ public function providerComputedConstraintName() {
   -      ['user_field_data', 'a_veeeery_veery_very_super_long_field_name__key', 'drupal_BGGYAXgbqlAF1rMOyFTdZGj9zIMXZtSvEjMAKZ9wGIk_key'],
   +      ['user_field_data', 'a_very_very_very_very_super_long_field_name__key', 'drupal_WW_a8TlbZ3UQi20UTtRlJFaIeSa6FEtQS5h4NRA3UeU_key'],
   

   I confirmed with wc that the old key was 48 chars and that the new one is 49, so this test is still working as intended:

   [ayrton:drupal | Mon 10:29:33] $ echo "a_veeeery_veery_very_super_long_field_name__key" | wc
          1       1      48
   [ayrton:drupal | Mon 10:29:37] $ echo "a_very_very_very_very_super_long_field_name__key" | wc
          1       1      49
   

Committed to 10.1.x. Thanks!

I queued tests against 10.0.x and 9.5.x to make sure the same change set works on those branches before backporting it.

CSpell passed in the above 10.0.x test results' console log, so I went ahead and cherry-picked this to 10.0.x. However, the patch does not apply to 9.5.x, so we need a separate 9.5.x. backport. Thanks!

Better status for the backport version.

Rerolled for 9.5.x, the major changes were related to ES6.
https://www.drupal.org/node/2815083

yarn build:js loves your work, sadly yarn prettier has a different opinion...

😅😅
thank you, I hope the core script likes my work now XD

empty files are not helpful, sorry....

Changes look good!

Good work!

I noticed the patch changed parameter naming for TermStorage/TermStorageInterface.

Since PHP 8.0 parameter naming are a part of the API contract, as named arguments may be used by callers.

// Will break
\Drupal::entityTypeManager()->getStorage('taxonomy_term')->getNodeTerms(nids: [], vocabs: []);

As much as its unlikely to happen in this case, as it is the first non optional, it is still possible and will break usages. I've certainly made use of parameter names when its redundant, in the name of legibility.

How can we make sure parameters are not affected by these spelling issues?

That's an interesting point @dpi and we might need to change our practices for it. Since it's just that one change we need to be concerned about, let's open a followup issue to discuss whether we should not allow parameter names to change for cspell and how to handle BC for them generally in a second policy issue.

I opened #3335354: [policy, no patch] Decide how to handle parameter renaming now that PHP 8 allows it to be used as an API to discuss BC for parameter names.

Committed and pushed the backport to 9.5.x. Thanks!