Problem/Motivation

Following #3154665: Disable spell checking within Drupal\filter\Plugin\migrate\process\FilterID::getSourceFilterType() I regenerated the dictionary and noticed two words were removed:

  • emtity
  • linktitle

"emtity" is a spelling error - one of the module names is misspelled.

"linktitle" is a correctly spelled module name that can be removed from the dictionary.

Proposed resolution

Fix the spelling error and remove both words from the dictionary.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3155463-5.patch2.15 KBquietone
#2 3155463.patch1 KBlongwave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Active » Needs review
FileSize
3155463.patch1 KB
quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented 
+++ b/core/misc/cspell/dictionary.txt
@@ -972,7 +971,6 @@ linkback
-linktitle

Why is this removed here?

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented

Because "linktitle" only exists in the ignored section of code in FilterID.php, so it can be removed from the dictionary.

In #3154669-9: Fix or ignore 33 words only used in the PostgreSQL driver I tried an alternative approach of ignoring words, I am starting to think that explicitly listing the words to be ignored for an individual file is cleaner and perhaps something we should do here as well?

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
FileSize
3155463-5.patch2.15 KB

@longwave, I read the comments in the issue referred to. It seems to me that if you are going to add spellchecking then you would want to maximize the coverage. So, having ~100 lines ignored by the spell checker seems like a step backwards to me. So, I made a patch to see what it would look like. It does add some visual clutter but that doesn't outweigh the benefit. Just my 2c.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Thanks for adding your opinion and uploading the patch.

Really not sure which is the best approach here, so going to mark this as RTBC and let core committers decide which route to go down.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer commented

As a reference: The doc of cSpell:ignore: https://github.com/streetsidesoftware/cspell/tree/master/packages/cspell...

Using cSpell:ignore is cleaner than using cSpell:enable/disable and its variations.

But it's harder to get rid of words if code were changed, and the word(s) should be removed from the ignored ones.

For maintainability, -1

Or adding above to the line closing to where words show up the first time

Quoting my comments on #3154669: Fix or ignore 33 words only used in the PostgreSQL driver -1 to #5

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer commented

Ways to bypass cspell checking

  1. File level: Ignore whole file: Adding the file path to ignorePaths in .cspell.json
  2. Word level: cSpell:ignore, applicable to words
  3. Lines level: cSpell:disable/cSpell:enable, applicable words spanning multiple lines
  4. Line level: cSpell:disable-next-line applicable to the line next

My suggestion is to use them where applicable the most. all 4 ways are encouraged to use. but do not encourage using cspell:ignore as the following way. Instead, we should use cSpell:disable/cSpell:enable, cSpell:disable-next-line and cSpell:ignore.

+++ b/core/modules/filter/src/Plugin/migrate/process/FilterID.php
@@ -13,6 +13,12 @@
+// cspell:ignore abbrfilter adsense autofloat biblio cincopa codefilter
+// cspell:ignore commonmark deepzoom emogrifier emptyparagraphkiller forena
+// cspell:ignore gotwo htmlpurifier htmltidy intlinks intlinks lazyloader
+// cspell:ignore linktitle multicolumn multilink mytube openlayers opengraph
+// cspell:ignore sanitizable shortcode shortcode typogrify wordfilter xbbcode
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Version: 9.1.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed

I think #5 is good because then you get more spell checking. With #2 we could remove emtity from the dictionary and we would not find the spelling mistake - because cspell has been disabled. Therefore #5 is better.

Committed 5f504df and pushed to 9.1.x. Thanks!

Backported the actual speeling fix because it makes migrations better.

Committed and pushed 297f6a87df to 9.0.x and 21f227387b to 8.9.x. Thanks!

  • alexpott committed 297f6a8 on 9.0.x 
    Issue #3155463 by longwave, quietone, jungle: Fix spelling error in...

  • alexpott committed 21f2273 on 8.9.x 
    Issue #3155463 by longwave, quietone, jungle: Fix spelling error in...

  • alexpott committed 5f504df on 9.1.x 
    Issue #3155463 by longwave, quietone, jungle: Fix spelling error in...

Status: Fixed » Closed (fixed)

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