Problem/Motivation
As proposed by @quietone in #3337327: CSpell dictionary is out of sync, we want to do a CSpell on all files when CSpell related files change.
The place to make this happen would be in web/core/scripts/dev/commit-code-check.sh
Steps to reproduce
Proposed resolution
- Determine which changes in which files should trigger a CSpell run on all files.
- Implement this in web/core/scripts/dev/commit-code-check.sh
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3338155
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3338155-cspell-all-files
changes, plain diff MR !10812
Comments
Comment #3
spokjeComment #4
quietone commentedI thought that this was fixed but on doing some research I see that I did an incorrect reroll at #3328741-28: Add a dictionary for Drupal-specific words.
The error was to remove the check on
core/.cspell.json. That should be restored.Anything else?
Comment #5
spokjeUnsure if this issue is still valid/needed?
Comment #6
quietone commentedI think we need to spell check core when
"core/.cspell.json"changes, which isn't currently done.Comment #9
jonathan1055 commentedI have made the suggested change to check if core/.cspell.json is one of the files that is changed.
First test just changing one other real file - all others are taken from cache. One failure reported as expected
https://git.drupalcode.org/issue/drupal-3338155/-/jobs/3899895
Make a change to .cspell.json - every file is tested (not from cache) - still only 1 error, as expected
https://git.drupalcode.org/issue/drupal-3338155/-/jobs/3899954
Undo the addition of the incorrect word - all files checked, all pass
https://git.drupalcode.org/issue/drupal-3338155/-/jobs/3900011
Remove one word from the core dictionary - all files checked, 76 occurrences reported
https://git.drupalcode.org/issue/drupal-3338155/-/jobs/3900133
Put back the dropped dictionary word and add 'unusual' to the flagwords. This shows that only a change to .cspell.json (and not the dictionary) is sufficient to test all files. Cache is not used, and six words reported as expected
https://git.drupalcode.org/issue/drupal-3338155/-/jobs/3900261
Temporary flagwords change removed. All files read from cache. Back to green.
https://git.drupalcode.org/issue/drupal-3338155/-/jobs/3900371
As a side note, I found it quite tedious to discover which files and words were being reported. With 15,800 file names beig displayed (you need to view the whole log) it is not easy see them. I had the advantage that I knew what the mis-spelled word(s) were that could search for, but if this was an ordinary pipeline job, and I had made a typo it would take internal knowledge of the cspell message text to be able to find them. A really nice improvement would be if the cspell detects any words than it is run again, without displaying the file names of every file being checked, and also has the --suggest option added, so it is easy to fix the errors. But that is for another issue.
This is ready for review.
Comment #10
jonathan1055 commentedI wanted to check the existing behavior, with a change to .cspell.json but not to commit-code-check.sh
and I was surprised to see that all files seem to be checked.
https://git.drupalcode.org/issue/drupal-3338155/-/jobs/3900589
Then I realised that this script is not controlling the pipeline cspell job at all, it is for local use only. So all the testing above was pretty much a waste of time :-) The pipeline job already checks all files when .cspell.json is changed.
Still, this is now done for local use too, and is ready for review anyway.
Comment #11
smustgrave commentedSeems pretty straight forward and makes sense to run.
Comment #12
jonathan1055 commentedThanks @smustgrave
I would like to use this MR for another spelling check. Then I will restore and put back to RTBC.
Jonathan
Comment #13
jonathan1055 commentedI've removed the test files added in #12, so back to the state of RTBC from #11
Comment #16
longwaveDefinitely makes sense to check the config file as well.
Committed 2264713 and pushed to 11.x. Thanks!