backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 39 forks source link

CSpell: Consider using the `--allow-compound-words` option #6260

Open klonos opened 11 months ago

klonos commented 11 months ago

This is a follow-up to #4537 in an effort to optimize the tasks our PRs are running. In the PR for that issue, when trying to address cspell tripping over the word pageviews, I have elected to correct the two instances in our codebase to page views instead (these were in docblocks by the way - not in actual code, so that was easy/safe). More generally though, there are various instances in our codebase that are compound words which are tripping cspell, and which they wouldn't if they were split using spaces, dashes, or underscores as appropriate per case.

https://github.com/klonos/backdrop/blob/1.x/.cspell/backdrop.dic is currently 571 lines long (and constantly growing obviously), and it includes at least 234 entries (about 41% of the total) that are compound words:

activemenu
admintheme
allowcache
allowedtags
attributename
backdropcaption
backdropcms
backdropfolder
backdropimage
backdropimagecaption
backdroplink
backdroptest
backdropunlink
Barepaths
basenames
basethemes
bitmasks
bitstring
bulkupdate
bulletedlist
caseflip
changedme
changethis
clickdown
clicksort
codefilter
colorindex
columnname
compiledquery
COOKIEFILE
COOKIEJAR
COOKIESESSION
correcthorsebatterystaple
datafield
datastructure
datatypes
datelimits
doubleclick
driveletter
droppables
editbackdropimage
elementtype
emaillog
enddate
endskip
entityreference
Entityreference
ENTITYREFERENCE
errorcode
feedburner
fetchmode
Fieldby
fieldlabel
fieldsql
fileapi
filefield
filelisting
fileurl
filterlabel
firstcolumn
firstsegment
FOLLOWLOCATION
footerborder
footertext
formdata
formfocusborder
formtest
foundkeys
fourthcolumn
fullname
groupinfo
groupnames
Hammertime
hovermenu
htmldog
Htmlto
HTTPAUTH
HTTPGET
imagefield
imagetests
imageto
infosystems
infourl
justifyblock
justifycenter
justifyleft
justifyright
keyfound
keyup
lastsplit
linkname
linknid
linktitle
linktype
litespeed
localzone
maxadjustment
maxdate
menutoggle
merlinofchaos
metacharacters
microweights
mimeheaders
minadjustment
mindate
modulename
mousebutton
mousedown
multibundle
multisites
myclass
mydata
myeditor
myfield
myfragment
myfunctions
myitem
mykeyword
mynewpassword
myplugin
myprofile
mystring
mytab
mytable
mytokens
mytype
myusername
newpath
newranges
newtext
nextdate
nextfirst
nextsecond
noparse
noscheme
nosuchcolumn
nosuchindex
nosuchscheme
nosuchtable
nosuchtag
notpromoted
notpublished
numberedlist
offsettopchange
oldserver
oldtext
oneof
onloadalert
otherjob
othername
pagedoesnotexist
pageload
pagerpieces
pastefromword
pastetext
phptemplate
phpwiki
portseparators
primarytabs
primarytabstext
propertyname
Punycode
quicktime
readwrite
realelement
runlength
screenreader
searchexpression
secondcolumn
secondconfig
selectmenu
shiftwidth
shortformat
shortnames
showblocks
sidebarborders
simpletests
skipdots
smartmenus
softtabstop
somedelta
somepath
someplugin
sometable
Somethis
stackoverflow
stepview
subarray
subpatterns
subrow
subtab
subtabs
subterm
superglobal
tablealias
tabledragging
tableresize
tablesorting
tabletools
tagname
testconfig
testfiles
testimage
testpage
testviews
textgroups
textinput
thirdcolumn
timeentry
timestep
titleslogan
todate
toolkitname
topoffsetchange
treeparent
treetable
treeterms
typeflag
unpackage
uploadimage
uploadwidget
USERAGENT
userinterface
valueadjustment
valuedate
VERIFYHOST
VERIFYPEER
verygreatbackdropmodule
VIEWNAME
watchdog
withslash
Wonderwoman
wordlength
workkeys

If not all of these, then most of them could be ignored if we used the --allow-compound-words global option for cspell (and there's also cspell:enableCompoundWords and cspell:disableCompoundWords if we wanted to do that per-file/per-line).

Pros:

Cons:

Alternatively, we could establish a policy to always use underscores where possible for variables and array keys etc. ...so it should be my_keyword and $my_new_password instead of mykeywordor $mynewpassword. Then we could go and change these things in our codebase accordingly (where applicable/safe).

The other option is for us to do nothing about this (just keep adding words to our custom dictionary as cspell complains about them in various PRs), in which case this issue can be closed.

quicksketch commented 11 months ago

I think of cspell as a tool to prevent most misspellings but not everything. It still won't catch grammar errors or similar sounding words (i.e. there/their/they're). That's fine. It catches most typos and that's what I think it's best at. So I would be in favor of enabling this setting and trimming down the allowed words dictionary.

Alternatively, we could establish a policy to always use underscores where possible for variables and array keys etc.

I think that is already the case? Or maybe it's a loose convention that derives from the variable naming policy. I would be in favor of this too.

klonos commented 11 months ago

Hmm 🤔 ...the cSpell.allowCompoundWords definition is listed in the "legacy" section in https://streetsidesoftware.com/vscode-spell-checker/docs/configuration/#legacy with the following note:

Note: this can also cause many misspelled words to seem correct.

In https://github.com/streetsidesoftware/vscode-spell-checker/issues/1069 one of the maintainers mentioned the following while closing the issue:

allowCompoundWords is a mixed bag. Lot's of issues are raised because of missed spelling issues: https://github.com/streetsidesoftware/vscode-spell-checker/issues/802, https://github.com/streetsidesoftware/vscode-spell-checker/issues/560, https://github.com/streetsidesoftware/vscode-spell-checker/issues/345, https://github.com/streetsidesoftware/vscode-spell-checker/issues/324, ...

By design, it only compounds words of at least 3 characters, otherwise many more missed spelled words would be considered correct.

Going forward, the idea is to have a dictionary of common compounded words and to turn off allowCompoundWords by default.

PS: also noted in https://cspell.org/configuration/document-settings/#enable--disable-compound-words :

Note: Compound word checking cannot be turned on / off in the same file. The last setting in the file determines the value for the entire file.

I'm having second thoughts 🤔