[ONGOING] CSpell: Fix nags and clean dictionary up

[klonos]

This is a follow-up to #6255, where we've added words to our dictionary, and fixed bits of our codebase that were tripping CSpell during our automated tests. It seems that we have now gotten in a good place where there are very rare occasions where PRs fail the CSpell check.

This issue here is to serve as a place where people can report any further CSpell failures, and then to also file small follow-up PRs for these main tasks:

1. Fix any CSpell nags in new PRs

The preferred action will be based on the following logic:

1. If the word exists only in a 3rd party library that we are using and never or very rarely change, then consider ignoring the entire file/path in the ignorePaths section of our .cspell/cspell.json configuration file. Using cspell:disable-next-line or cspell:disable/cspell:enable pairs in these cases is impractical, as we shouldn't be changing 3rd party code (we'll loose those ignores if the library is updated in the future).

2. If the word is non-English (testing Unicode for example), the preference will be to ignore it with cspell:disable-next-line, or with cspell:disable/cspell:enable pairs when dealing with multiple lines where these words occur.

3. If the failure is for existing variable/function names or comments, then try to fix/rename those where possible/practical instead of adding more words to our dictionary - especially when these words are used only say 2-3 times in our entire codebase.

4. If the failure is for newly-introduced words in the PR, then:

   • try to fix those (split any combined words with dashes or underscores, or use camelCase for example)
   • or, come up with alternative words that do not trip CSpell
   • or, if the word is a made-up, fictional word used for testing purposes or as a documentation example, then see any existing fictional/test words in our dictionary can be used instead (if you are calling the thing pigglywiggly for example, then rename it to foobar which we have already added to our dictionary).
   • or, if the word occurs multiple times in a single file, but you don't want to ignore the entire file, nor use dozens of cspell:disable-next-line, then consider ignoring the specific word(s) in the specific file, by adding an entry in the overrides section of our .cspell/cspell.json configuration file.

   Again, the goal will be to avoid adding more words to our custom CSpell dictionary where possible.

5. When fixing the codebase is not practical/possible, new words may be added to the dictionary. This will be able to happen without any core commit once we have moved our custom dictionary to its dedicated repo in https://github.com/backdrop-ops/cspell (refer to #6280).

2. Clean up our current list of words in the custom CSpell dictionary

The biggest offenders are combined words, like thingname instead of thing_name or thing-name or thingName that are acceptable by CSpell. This is an actual list of existing words in our dictionary:

myclass
mydata
myeditor
myfield
myfragment
myfunctions
myitem
mykeyword
mynewpassword
myplugin
myprofile
mysiam
mystring
mytab
mytable
mytokens
mytype
myusername
myverylongurlexample
myverylongurl
...
newpath
newranges
newtext
nextdate
nextdatealt
nextfirst
nextsecond
...
noscheme
nosuchcolumn
nosuchindex
nosuchscheme
nosuchtable
nosuchtag
nothere
notpromoted
notpublished
...
otherjob
othername
otherval
...
somedelta
somepath
someplugin
sometable
...
testconfig
testfiles
testimage
testpage
testparam
testviews

You get the picture 😞

Lets try to rename those if they are used in a single place ("self-contained" inside the same function for example), and then remove them from the dictionary.

[bugfolder]

Getting CSpell to the point where it is adding more value than "crying wolf" has been a great accomplishment, and I wholeheartedly support the continued cleanup. It seems, then, that we should have a section in our Coding and documentation standards that mirrors the "preferred actions" above. Some of that is covered in Naming Conventions, but it would be helpful to developers to have documented the CSpell annotations to deal with things that might get flagged.

So as not to derail this issue with further discussion of the documentation topic, I've opened an issue over in the backdropcms.org issue queue: https://github.com/backdrop-ops/docs.backdropcms.org/issues/231.

[klonos]

Although I never had this happen while filing all the different PRs for #6255, I always wondered how come all the // cspell:* ignore lines we have been adding were not tripping PHPCS about those comments not ending with a period. Well, I now had this come up in one of my PRs:

Since PHPCS is not complaining about these comments starting with a lowercase letter, I've filed a PR that only adds periods for now: https://github.com/backdrop/backdrop/pull/4580

Unfortunately, PHPCS is choking on too many changes, so we won't be able to tell if it is happy now 😞 🤷🏼

[klonos]

Unfortunately, PHPCS is choking on too many changes, so we won't be able to tell if it is happy now 😞 🤷🏼

Yes we can! ...here's a test PR that only changes a single // cspell:enable line (the same one that tripped PHPCS in the other PR I posted a screenshot for in my previous comment) to add a period to it: https://github.com/backdrop/backdrop/pull/4581

PHPCS didn't complain, so I guess it's just the periods at the end that are tripping it. ...we did get the usual LayoutUpgradePathTest gremlins, but nothing else failed.

[klonos]

...I've filed this issue to discuss if the cspell ignore comments should be excluded/ignored by PHPCS: https://github.com/backdrop-ops/phpcs/issues/16

If that is possible, it would make things easier. Nothing to change in the way we have been adding cspell ignores.

[quicksketch]

@klonos Two updates here:

• I merged https://github.com/backdrop/backdrop/pull/4583 to fix the existing typos and expand the dictionary a little further.
• I closed https://github.com/backdrop/backdrop/pull/4580, since this is a problem with our phpcs rules and should instead be fixed in https://github.com/backdrop-ops/phpcs/issues/16.

I'm not sure if we want to keep this issue open, since there are now no direct action items here. Please reopen if needed.

[klonos]

Thanks @quicksketch ...reopening this in order to fix more nags that have come up in recent PRs.

I'm making this an ONGOING issue.

[klonos]

I just filed a single PR to change instances of mymodue to my_module instead: https://github.com/backdrop/backdrop/pull/4654

Since that has been used extensively for code examples throughout all our *.api.php files, there are plenty of lines changed over 24 files. To make it easier to review, I decided to create a single PR just for this case.

Note: Our CSpell configuration allows for camel case (for JS code and PHP class names etc.), so things like myModule or MyModuleClassXyz were not caught. MYMODULE is still caught, so I cahnged that (only a single instance of that by the way),

[quicksketch]

I merged https://github.com/backdrop/backdrop/pull/4654 into 1.x and 1.27.x. I removed the tag and set this back to open, per @klonos's request to just keep using this issue for every round of cspell fixes.

[klonos]

Here's another PR to fix nags with CSpell (and some PHPCS) that have been popping up in recent PRs: https://github.com/backdrop/backdrop/pull/4707

[klonos]

Noting that I've filed an upstream PR with CSpell that adds any words starting with un from our dictionary to the official US dictionary used by CSpell: https://github.com/streetsidesoftware/cspell-dicts/pull/3129

If that gets accepted, we should be able to remove those words from our dictionary 😉

[quicksketch]

I appreciate your optimism :grinning: "unhighlight" though... lol

I don't know what else you would call that. We use a lot of words in computing that don't make sense in the real world.

[quicksketch]

I merged https://github.com/backdrop/backdrop/pull/4707 into 1.x and 1.27.x. Thanks for those fixes!

Continuing to leave open, I think there's still quite a bit of cleanup needed here.

[klonos]

Thank you @quicksketch 🙏🏼

Keeping this issue open, since it is an on-going task.

[klonos]

@backdrop/core-committers here's another PR that fixes common nags in recent PRs: https://github.com/backdrop/backdrop/pull/4714

[quicksketch]

Thanks @klonos! Newest PR is also now merged.

[klonos]

Thanks @quicksketch 🙂

Since this is an on-going task, reopening and resetting issue labels, milestone etc.

[klonos]

I've filed another PR, but not marking it as ready for review/merge yet, since only a couple of words were fixed. Waiting for some more commonly-flagged words to come up in PRs.

[quicksketch]

I've filed another PR, but not marking it as ready for review/merge yet, since only a couple of words were fixed.

I went ahead and merged your PR (https://github.com/backdrop/backdrop/pull/4719) anyway. Very minor and I was seeing those cspell issues frequently.

Leaving open for more PRs as needed.

[klonos]

Started another PR that fixes nags in recent PRs: https://github.com/backdrop/backdrop/pull/4840

I'll go through some more PRs and try to fix any nags in them, and will mark this as ready for review once it has a decent set of fixes.

[klonos]

OK, PR ready for review. Once merged, it should fix the nags in a dozen or two of recent PRs in the queue after these PRs are rebased.

[quicksketch]

I merged the most recent PR at https://github.com/backdrop/backdrop/pull/4840 into 1.x and 1.28.x. Thanks again! Reopening for more ongoing fixes.

[quicksketch]

New PR filed at https://github.com/backdrop/backdrop/pull/4902 that fixes some nags I've gotten on recent PRs.

[quicksketch]

Thanks @herbdool, merged that set of changes and I'm setting this back to an open issue.

[quicksketch]

Another PR fixing a set of spelling issues: https://github.com/backdrop/backdrop/pull/4906

[herbdool]

@quicksketch I updated the PR's branch from 1.x and now it's complaining about a handful more words like "opendocument".

[quicksketch]

Thanks, seems like we should ignore all file.type.*.json files, since they mostly contain lists of mimetypes that are notorious for run-on words. PR is updated.

[herbdool]

Looks good now @quicksketch

[quicksketch]

Great, thank you @herbdool! Merged into 1.x and 1.29.x.

[quicksketch]

I merged https://github.com/backdrop/backdrop/pull/4935 into 1.x and 1.29.x that adds a few more words to the dictionary based on recent PRs. This set was trivial so I merged it after ensuring tests passed.

[quicksketch]

I added "multipass" to the dictionary in https://github.com/backdrop/backdrop/pull/4940. That seems to be the one occurring most frequently yet.