Problem/Motivation

It is not easy to figure out how to make a sensible set of words to change. As I thought about it I decided that it was worth trying to fix words that appear 1 time in 1 file. This set of words are most likely to be very simple fixes which will be easy to review. It also means that there are less changes to be made to remove a word and thus the size of dictionary.txt can be reduced significantly for minimal work. There are around 900 words like this. Although I just found an error in the way I generated that list and I have to do it again.

All these words are used in one file. I started with 'a' and while editing spotted some words not beginning with 'a' that are used in just one file. By my estimates word beginning with a -> d that are used once represent about 20% of the total words that are used once. Some words that are used once, such as dublincore*, are already in #3210125: Fix spelling in core.* yml files, so are excluded here.

The list of words beginning with a -> e, inclusive, that are used once in one file.

Proposed resolution

Limit this to one line easy fixes, avoid any possible controversy, or any change that would better fit with other words, such as doing chien and chiens in the same patch.

Add more starting letters but keep the < 50k for easy review.

Remove the following from dictionary.txt.
42 words
-abempty
-abiword
-aewesome
-aflopend
-aliasable
-aliquet
-allowtransparency
-apng
-asubject
-autodiscovered
-autofilling
-autoincrement
-autoincrementing
-autoindex
-baseroot
-bistromathic
-blackhat
-bzzzzzzzt
-cacheablemetadata
-clicksort
-closur
-configtranslation
-contrained
-crudui
-crypted
-currenttime
-currentuser
-datefield
-daycounter
-defalt
-dependening
-displayname
-distincted
-driveletter
-elit
-meφω
-oplopend
-sorteren
-toepassen
-vivamus

Remaining tasks

  • Scoping of issue has been agreed to by xjm. See #14.
  • Review
  • Commit

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

Not yet testing. I need to do a self review.

longwave’s picture

I have a feeling this will be rejected for scoping reasons, the same way we don't do things module-by-module, but I could be wrong.

We might be OK with e.g. fixing all of views.view.*.yml the same way the core.*.yml issue is quite self-contained.

quietone’s picture

Issue summary: View changes

Yes, finding a set of related words, self-contained, as you say is helpful and that is why I did the 'migrate' words. I was going for an easy win that would reduce the number of words in the list quickly. Sort of getting the 'noise' out of the file.

I think I'll ask the committers to comment.

quietone’s picture

Issue summary: View changes

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

I asked xjm to about the scope on this in #contribute.
The opinion was that at 13K the current patch is too small but it partly depends on the patch author and reviewer burden. To answer those questions I will add the letter 'b' and see how long it takes and consider the reviewer perspective.

First, reroll the existing patch.

quietone’s picture

Title: Fix spelling for words used once, beginning with 'a' » Fix spelling for words used once, beginning with 'a' or 'b'
Issue summary: View changes
FileSize
3.16 KB
16.86 KB

Adding 'b'. My old list of words identified 21 that are used once in one file of which 5 have been removed in this patch. The ones not changed were those likely to be considered words, anything to do with javascript, proper name or for other reasons caused more changes. I am really trying to keep this to words the require a single code line change.

From a patch builder perspective finding these one off changes is, for me, as challenging as finding some other grouping of files or words to do.

No comment yet on reviewer perspective - I need to do something else for while.

quietone’s picture

Title: Fix spelling for words used once, beginning with 'a' or 'b' » Fix spelling for words used once, beginning with 'a' -> 'c', inclusive
Issue summary: View changes
FileSize
9.01 KB
25.99 KB

Adding 'c'.

quietone’s picture

Title: Fix spelling for words used once, beginning with 'a' -> 'c', inclusive » Fix spelling for words used once, beginning with 'a' -> 'd', inclusive
Issue summary: View changes
FileSize
6.63 KB
32.87 KB

I need to review the work I have done tonight. I'll run tests after that.

quietone’s picture

Issue summary: View changes

Putting my reviewer hat on I started to review the patch.

I immediately noticed that I like that the fix for each word is a one line change (perhaps more if the wrapping of a comment changed). As soon as one word is finished I know that I will not encounter it later in the patch. Sometimes that means referring back to the earlier file to check wording or something. That is avoided. Those are the only differences I see. Everything else is the same, the reviewer needs to run spellcheck on core locally and check all the added ignore lines for alphabetizing and wrapping.

The patch is currently 32KB. My feeling is that the max should be 50KB. Any other opinions on that?

quietone’s picture

Status: Active » Needs review
quietone’s picture

Issue summary: View changes

Only 48 words, not 50

xjm’s picture

Yeah I agree that we definitely don't want patches to hit the 50K mark, so we probably shouldn't do more than three letters at a time unless one of them is "X" or something.

I'm okay with "letters starting with [_] through [_]" as a way to scope these issues, since one at a time would be a ton of overhead but the whole alphabet at a time would be unmanageably large. It is better than per-module scope because the scope is completely resolved for the words included.

Thanks @quietone!

quietone’s picture

Issue summary: View changes

@xjm, thank you.

I've updated the remaining task in the IS to show that this approach has committer approval.

I think this is ready for review now.

quietone’s picture

Issue summary: View changes
FileSize
1.34 KB
quietone’s picture

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
quietone’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
4.23 KB
31.91 KB

Rerolled and then ran `yarn spellcheck:core`.

Spokje’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

- Some nice catches in there ("dependening" anyone?)
- Great that it catches stuff like cacheablemetadata vs CacheableMetadata
- Green TestBot

RTBC for me.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Some patch conflicts to sort out... core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php will have moved to the new sqlite module.

murilohp’s picture

Status: Needs work » Needs review
FileSize
30.7 KB
6.38 KB

Hey, I made a new reroll of patch #20 and after ran yarn spellcheck:core, I got the following issues:

yarn run v1.22.5
$ cspell "**/*" ".*" "../composer/**/*" "../composer.json"
drupal/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php:171:44 - Unknown word (AUTOINCREMENT)
drupal/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php:550:38 - Unknown word (autoindex)
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

The file drupal/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php was added recently on #3129043, so I had to rollback the following words inside core/misc/cspell/dictionary.txt

  • autoincrement
  • autoindex

Moving back to NR to see what do you think about it.

Thanks!

murilohp’s picture

Issue tags: -Needs reroll
quietone’s picture

@murilohp, thanks for the reroll and for running spellcheck on all of core. The new file core/modules/sqlite/src/Driver/Database/sqlite/Schema.php was originally /core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php and was moved as part of #3129043: Move core database drivers to modules of their own. The patch in #20 had a cspell ignore line and that should be moved as well, since the goal is to remove words from dictionary.txt

This patch restores the ignore line in the sqlite Schema.php file.

Spokje’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed ff2d2ea and pushed to 10.0.x. Thanks!
Committed 3310654 and pushed to 9.4.x. Thanks!

I had to resolve some conflicts on 10.0.x in core/tests/Drupal/FunctionalTests/Bootstrap/UncaughtExceptionTest.php but nothing too tricky - a spelling mistake had already been removed due to not supporting PHP 7.

  • alexpott committed ff2d2ea on 10.0.x
    Issue #3210129 by quietone, murilohp, xjm: Fix spelling for words used...

  • alexpott committed 3310654 on 9.4.x
    Issue #3210129 by quietone, murilohp, xjm: Fix spelling for words used...
quietone’s picture

Issue summary: View changes

Fix typo in IS>

Status: Fixed » Closed (fixed)

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