Improve the cspell exception in Drupal.Commenting.InlineComment.InvalidEndChar

Oops forgot to link the docs in the issue summary.

Just ran into this with core. It is certainly a nuisance that only 1 spelling of cspell is recognized.

In core the following are also detected and reported as errors.

// cspell:disable
// cspell:disable-next-line
// cspell:ignore·fffffg
// cspell:ignore·tést·fïle·nàme

This, // cspell:ignore·fffffg, fails when on line 719 on the file but not on line 19. I copied and pasted the line. And similar with cspell:ignore·tést·fïle·nàme it fails on line 101 but passes on line 13. And the // cspell:enable after the disable mentioned about is not reported as an error. But if I move the 'disable' a line down, it is not detected. For these cases, maybe it because the line before is also an in-line comment and coder thinks it is more text.

Since this is detecting valid spellings of cspell as errors as well as valid cspell lines as error, I am changing this to a bug.

It is also blocking core issue #2719663: Fix 'Drupal.Commenting.InlineComment.InvalidEndChar' coding standard →

Thanks for reporting, I pushed a fix!

For reference, this is the commit you made
https://git.drupalcode.org/project/coder/-/commit/b4972e78ac25e514ee00e1...

I think there were actually two problems reported in the issue summary, and only one has been dealt with here.

The change from the specific strpos($commentText, 'cspell:') to the more general preg_match('/(cspell|spell\-checker):ignore/i', $commentText) certainly solves the problem. But this, technically, is too relaxed. It means that if 'cspell:ignore' or 'spell-checker:ignore' are found anywhere in the comment then the comment will be ignored. So if 'cspell:ignore' was put in the middle of a line, say when refering to it in text, that would still be matched. I know this is a rare edge case, so maybe its OK. But is does also mean that the actual real comment text is not checked for the end char. For example

// This comment is now not checked for valid line end char 
// cspell:ignore bananarama

Setting back to 'needs work' so that the issue does not automatically close just yet, to allow the discussion to continue. I can help on the sniff, I am not saying you have to do the work :-)

Also as per the cSpell documentation, we should allow 'spellchecker' without a hyphen. And we should remove 'ignore' from the regex, because this change should also cater for 'disable', 'disable-next-line' and 'enable'. So just having ':' at the end of the regex pattern should be sufficient.

Thanks, good points.

For the too relaxed edge case: I think we don't care. I assume it would make the sniff code more complex to detect/split comments. But I'm very happy to review and accept pull requests that try to address this!

For the additional keywords: I think your proposal makes sense to just match for the colon at the end of the regex pattern and add "spellchecker". I will check if I can do that quickly now.

OK, that was quick and we already changed the regex to only match for the colon.

Closing this as fixed again, please make a new issue if you want to address the too relaxed edge case :-)

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