cspell does not flag "ist" as a typo

Adding to flagWords for now. Let's see if this breaks anything.

With the patch applied, repeated the steps from the summary and now get:

1/1 ./authorize.php 633.87ms X
/.../core/authorize.php:45:34 - Forbidden word (ist)
CSpell: Files checked: 1, Issues found: 1 in 1 files

CSpell: failed

So that would have caught the bug in 📌 Add the session to the request in KernelTestBase, BrowserTestBase, and drush Fixed . 🎉

Yay, bot is happy. So we’re not relying on “ist” to work anywhere else.

Fixing summary to move all the problem/motivation text together.

i added "ist" to ad file and deleted all the dictionaries from cspell.json and ran commit-code-check and 'ist' was not reported, unlike all the Drupalism.

Yes, upstream sounds good.

Thanks for clarifying, @quietone. However, not sure why you came to this conclusion:

Yes, upstream sounds good.

vs. adding "ist" to flagWords and being done (per the patch).

Fixing this upstream sounds like a long, painful process. 😬 Adding another entry to flagWords is trivial (and done). 😅

It still makes sense to notify upstream

Followed the same steps.

Added // ist to a file and ran commit-code-check with 0 issues.

Applied patch and ran the check and it was caught.

@dww, I mean that filing an issue upstream makes sense.

https://github.com/streetsidesoftware/cspell/issues/4432 😉

@dww, thanks for your patience with my communication challenges on this issue.

There was a response in the cspell issue which suggested using 'trace' to find which dictionary is using the word. It is 'cpp'.

$ yarn cspell trace ist
yarn run v1.22.19
--- trimmed
ist  * cpp                  node_modules/@cspell/dict-cpp/cpp.txt.gz
--- trimmed

But that dictionary is not enabled in cspell.json and if it were there were be a '*" after the dictionary name. It is more likely this is because of the default word length of 4. And that means this is not bug. Therefor, I am changing this to a task. Adding 'documentation updates' to the remaining task so the information on word length is added to the wiki page.

Then, should this change be made? Should spelling errors in words that are less that 4 characters always be put into flagWords? It makes sense for common typos like the current flagWord 'hte'. But is 'ist' a common mistake?

There are comments on the upstream issue, do we want to reduce the minimum word length to three (seems like it would start catching lots of acronyms) or should we add a suggestion to flagWords (which seems like a good idea).

Per the suggestion in the cspell issue I added "minWordLength": 3, to cspell.json. I then rebuilt the dictionary and have uploaded the results. There were 271 insertions(+), 2 deletions(-) to the dictionary. I don't see any reason to add that extra work. So, I agree with catch that we stay with the default minimum of 4 letter word.

As for using the suggestion, flagWord: ist->is. First I tried it with "hte->xyz", made the spelling in a file. The result was Forbidden word (hte) fix: (xyz) in the output. It is nice that there is a suggestion but someone still needs to read the code to be sure, so I'd rather not have a suggestion at all.

I think that answers the questions above.

Forgot to upload the file.

Can we have a test run with new option and fixed vocabulary

@andypost, I am not sure what you mean by 'new option'. Do you mean changing "ist" to "ist->is"?

Also, there is no vocabulary to fix here, this is just adding one line to flag words. There are also no changes to the dictionary.

Based on @quietones research in #14 seems patch #2 is still the preferred solution.

The last submitted patch, 2: 3355243-2.patch, failed testing. View results →

Unrelated failed testing.
I rerolled the patch in the MR as the affected list by the patch was updated.
Setting back to RTBC as agreed on the solution by the reviewers.

Committed and pushed to 11.x.

Does not backport cleanly to 10.1.x, not sure it really matters given we should catch any spelling issues in 11.x.

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