Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSpell file name linting does not use (custom) CSpell configuration #2058

Closed
Isalgeon opened this issue Nov 7, 2022 · 6 comments · Fixed by #2148
Closed

CSpell file name linting does not use (custom) CSpell configuration #2058

Isalgeon opened this issue Nov 7, 2022 · 6 comments · Fixed by #2148
Labels
bug Something isn't working

Comments

@Isalgeon
Copy link
Contributor

Isalgeon commented Nov 7, 2022

Describe the bug
The new capability for the CSpell linter to also lint file names does not appear to use the (custom) CSpell configuration.
Custom words that I've got in the ignored list, which lint just fine in my documentation files, fail for the file names.

To Reproduce
Steps to reproduce the behavior:

  1. Set up prerequisites:
    • Introduce a word in CSpell that is not in the out-of-the-box dictionary but is included in the (custom) CSpell configuration.
    • Don't configure the SPELL_CSPELL_ANALYZE_FILE_NAMES setting in the ML configuration (i.e. out-of-the-box experience).
  2. Run the CSpell linter.
  3. The CSpell linter reports the word as unknown in the /tmp/<some-generated-UID>-megalinter_file_names_cspell.txt.

Expected behavior
The CSpell linter must use the (custom) CSpell configuration for the new capability as well.

Example logs
Some example logs:

Results of cspell linter (version 6.14.0)
See documentation on https://oxsecurity.github.io/megalinter/latest/descriptors/spell_cspell/
-----------------------------------------------

❌ [ERROR] for workspace /tmp/lint
Linter raw log:
/tmp/2f2eb435-22dc-4aae-b15e-035eb0ba804a-megalinter_file_names_cspell.txt:1:21 - Unknown word (MegaLinterBestToolEver)
CSpell: Files checked: 1, Issues found: 1 in 1 files


You can skip this misspellings by defining the following .cspell.json file at the root of your repository
Of course, please correct real typos before :)

{
    "version": "0.2",
    "language": "en",
    "ignorePaths": [
        "**/node_modules/**",
        "**/vscode-extension/**",
        "**/.git/**",
        "**/.pnpm-lock.json",
        ".vscode",
        "package-lock.json",
        "megalinter-reports"
    ],
    "words": [
        "MegaLinterBestToolEver"
    ]
}


You can also copy-paste megalinter-reports/.cspell.json at the root of your repository

Additional context
The observation I had straight away is that the above-mentioned file is located in the /tmp folder instead of /tmp/lint like the rest. Perhaps that affects the way the configuration is loaded and/or used.

Another thing that might affect the situation is that I use multiple separate dictionary files (e.g. words.txt) in a .dictionaries folder within the root of the repository instead of specifying them in a gigantic array in the CSpell configuration file.

Related pull request: #2009.

@Isalgeon Isalgeon added the bug Something isn't working label Nov 7, 2022
@Isalgeon Isalgeon changed the title CSpell file name analysis does not use CSpell configuration CSpell file name linting does not use CSpell configuration Nov 7, 2022
@Isalgeon Isalgeon changed the title CSpell file name linting does not use CSpell configuration CSpell file name linting does not use (custom) CSpell configuration Nov 7, 2022
@Kurt-von-Laven
Copy link
Collaborator

Kurt-von-Laven commented Nov 26, 2022

The observation I had straight away is that the above-mentioned file is located in the /tmp folder instead of /tmp/lint like the rest. Perhaps that affects the way the configuration is loaded and/or used.

Good point. I know there has been some work on using relative paths recently, so maybe that will help with us.

Another thing that might affect the situation is that I use multiple separate dictionary files (e.g. words.txt) in a .dictionaries folder within the root of the repository instead of specifying them in a gigantic array in the CSpell configuration file.

Thank you for calling this out, but I suspect this is not a factor. I was able to reproduce this issue at MegaLinter v6.15.0 in ScribeMD/pre-commit-hooks#164. We use a single custom dictionary file, but even words like "markdownlint" that are present in the npm CSpell dictionary and "pyproject" in the Python dictionary, get reported.

@nvuillam
Copy link
Member

cspell looks by itself to configuration files, and as the temporary file with file names is written in a temporary folder, it does not have access to you defined config files

One option could be to copy the file ith file names at the root of the repo and not in a temp folder, but we have to make sure it is well removed after, else for users using auto-commit of fixes, it will commit a useless file ^^

nvuillam added a commit that referenced this issue Dec 20, 2022
…on (#2148)

* Fix CSpell file name linting does not use (custom) CSpell configuration

Fixes #2058

* [MegaLinter] Apply linters fixes

Co-authored-by: nvuillam <nvuillam@users.noreply.github.com>
@Isalgeon
Copy link
Contributor Author

Isalgeon commented Dec 20, 2022

Good work on fixing the underlying issue; thanks!
Hopefully, I can start using the functionality in the new year. 🙂

As an aside follow-up question, doesn't the auto-commit of fixes feature use the -u parameter to ensure only updated files are committed? If not, that could be a slight but worthwhile improvement for the feature.

@nvuillam
Copy link
Member

@Isalgeon you're welcome, you can already try it with MegaLinter beta version if you like (it has not been tested)
About -u parameter... it could be an option, be as MegaLinter is also used locally ( pre-commit, mega-linter-runner ... ) I prefer to make sure that no extra file at all remains within the repo after a MegaLinter run

@Isalgeon
Copy link
Contributor Author

@nvuillam, I've just tested the feature on the latest beta version, and it works perfectly! Happy holidays! ^^

@Kurt-von-Laven
Copy link
Collaborator

@Isalgeon you're welcome, you can already try it with MegaLinter beta version if you like (it has not been tested) About -u parameter... it could be an option, be as MegaLinter is also used locally ( pre-commit, mega-linter-runner ... ) I prefer to make sure that no extra file at all remains within the repo after a MegaLinter run

Yes, I could totally see extra files wrecking havoc for a subset of our users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants