Problem/Motivation
Some of the words are positioned in not alphabetical order.
Proposed resolution
Add test for sorting order of dictionary.txt file
Remaining tasks
Add test
Fix list sorting
Issue fork drupal-3336828
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:
- 3336828-file-dictionary.txt-is
changes, plain diff MR !3305
Comments
Comment #3
volegerCSpell/SortTest shows that the file contains a list that is not sorted correctly.
Adding changes to the list makes the test passing.
Comment #4
volegerComment #5
longwaveThe dictionary generator is supposed to sort the file, but the error proves that we must have merged some changes manually; so the test is good to ensure that it stays sorted.
A few nitpicks in the test but overall this looks great.
Comment #6
spokjeAren't we fighting the symptom here?
Maybe we can trigger a
yarn spellcheck:make-drupal-dictin commit-code-check.sh on all files if dictionary.txt changes?If the the new dictionary is different from the one in the patch/MR then fail and we should be fine?
Comment #7
longwaveyarn spellcheck:make-drupal-dictis quite slow, this test should be quick. I also think there are still some locale issues with the dictionary generator, as the accented characters sometimes change for reasons I don't yet understand, and regenerating on DrupalCI might trip this up further.Comment #8
longwaveHaving just written #7 it strikes me that the issue might be that we need to apply
LC_ALL=Cto thetrcommand as well as thesortcommand.Comment #9
volegerThanks for the review, I addressed all review comments.
Comment #10
longwaveThanks, this looks OK to me and will prevent regressions here in the future.
Comment #11
volegerRebased after #3337327: CSpell dictionary is out of sync
Comment #12
volegerComment #13
catchI was tempted to agree with Spokje that we could skip this step and go straight to #3338155: Run CSpell on all files when CSpell-related files change... but cherry-picks bypass core commit checks, so this should catch situations where we're introducing discrepancies between 10.1.x and 10.0.x (although only post commit so if it happens we'll need to hotfix it).
Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!
Comment #17
jonathan1055 commentedIf all the commits are done, can the MR be closed as it was not actually merged here.