Rename ColourDefinitionSniff to ColorDefinitionSniff

Keshav Patel → made their first commit to this issue’s fork.

Renamed ColourDefinitionSniff to ColorDefinitionSniff, Please Review.

I'm not a big fan of renaming sniffs as then this will break all phpcs configurations that exclude this sniff for example.

What could we do to preserve compatibility?

Also instead of doing this: How easy is it to make an exception in Drupal core in the phpcs.xml.dist config file so that the spell checker ignores this instance?

Side note: Coder is developed on Github, please create pull requests there once we come to a conclusion here. https://github.com/pfrenssen/coder/pulls

Setting to "needs work" to get some answers on my previous comments.

@klausi, sorry it too so long to get back. This was brought to my attention again because of a new core issue that I just moved to Coder. Like this one it about a sniff name that does not pass cspell checks. 🐛 cspell complains about phpcs sniff name Active .

It is true that an exception can be added although I have not yet included it in a spelling issue in core.

However, I still think that Coder itself should follow the Drupal spelling standard and change the name. In principle I think that is the correct action, even though I appreciate the challenges of renaming things and the work to deprecate. And now that there are two issues about the problem maybe this needs more thought?

On the other hand, I don't know how many current sniffs may violate the current core usage of cspell. Maybe the way to go is to just prevent future misspelling. Is there any practice in Coder to ensure that new sniffs follow American English and pass cspell checks?

I think enabling cspell checking in Coder is a very good idea, so that we at least prevent future spelling problems. Started a draft PR at https://github.com/pfrenssen/coder/pull/233 , not finished yet. Should probably be done as separate issue and then we can continue the discussion here what to do about already established sniff names and error codes.

Opened 📌 Enable cspell checking in Coder Active to track that.

Enabling cspell in Coder is an excellent and vital thing to do, to ensure no new sniff names are invalid. But I agree with klausi that renaming a sniff now will break more things than it solves. I did a quick codebase search for ColourDefinitionSniff and found 29 occurences - many of these are in /vendor/ and I also note that Squizlabs has the same English spelling. We cannot rename a sniff without breaking all of those projects.

I think the most realistic solution will be to add the two sniff names (or rather, the section of the sniff name that fails as a word) into one of Drupal's own spelling dictionaries https://git.drupalcode.org/project/drupal/-/tree/11.x/core/misc/cspell?r... These are used in the gitlab cspell job, so will be fixed for all contrib.

Renamed this issue. There are potentially six sniff parts that need to be added to the dictionary - see the list in #2 on #3465767-2: Enable cspell checking in Coder →

What about adding a correctly named sniff that is a wrapper for the incorrectly named one?

Also, how do I get this sniff to fail in core? I changed a color to upper case and it was not detected.

It looks like in the Drupal core phpcs.xml.dist config you forgot to add the CSS file extensions that you want to check.

<arg name="extensions" value="php,module,inc,install,test,profile,theme,css,info,txt,md,yml"/>

Maybe there is generated CSS in core that we don't want to check and that's why it was never enabled?

Wrapper sniff: that is an interesting idea, how could we do that without breaking existing configurations? Then both sniffs would run and duplicate the errors messages, could we prevent that?

Otherwise I agree with jonathan1055, we should add those names as exceptions to Drupal core's cspell config.

klausi → closed merge request !23

I have just tested the six sniffs and error codes that Klausi listed on #3465767-2: Enable cspell checking in Coder →

* SeletorSingleLine
* ColourDefinitionSniff
* TeamplateSpacingAfterComment
* UnecessaryFileDeclaration
* TforValue
* TrhowsCommentIndentation

Each word in a CamalCase string resets the search (this is probably obvious and known to you, but worth mentioning anyway). So the actual words to add to the core dictiionary would be the part words that are misspelled:

seletor
teamplate
tfor
trhows
unecessary

The English speling of colour is already in the core dictionary, so that does not currently fail the cspell checks in contrib.

My test job is https://git.drupalcode.org/issue/scheduler-3356800/-/jobs/3883429

Hm, but adding those partial words would be bad. Can we add the full words like "SeletorSingleLine" to the dictionary? Then it only matches this exact misspelling from Coder.

Yes you are right, that is better. Even though adding the short word is sufficient, adding the full sniff name/code will also match and not report the word.

Here's another test - in which I added 'SeletorSingleLine' and 'Teamplate' to the project dictionary. You can see that the long string 'TeamplateSpacingAfterComment' is not reported, showing that the short word 'teamplate' is sufficient (but we won't be adding that)

The short word 'seletor' in the test file is reported as wrong, but the full word 'SeletorSingleLine' is passed as OK, which is exactly what we want.

https://git.drupalcode.org/issue/scheduler-3356800/-/jobs/3884432

So it is the six long strings as listed in #17 above which are needed in the core dictionary. When that is done, we can remove the cspell:ignore added in the Coder files.

What would be the appropriate place to request these words to be added?

Probably Drupal core core/misc/cspell/drupal-dictionary.txt is the canonical home of the cspell dictionary? I would assume that one is used for gitlab runs https://git.drupalcode.org/project/gitlab_templates/-/blob/main/scripts/...

I didn't frame the question very well, what I meant was which issue queue should that request be made? Is it just a plain core issue? or infrastructure? It's not really a coding standards issue.

Both of the core files are used in the contrib gitlab job. It may be better to add them to the smaller drupal-specifc misc/cspell/drupal-dictionary.txt as it is Drupal source code which has created the incorrect spellings.

Drupal core changes will need a drupal core issue. All drupal core changes are handled in the drupal core issue queue, so best open one there.

I've updated the issue summary and changed the project to Core, instead of starting a new issue.