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

Check spelling of filenames #3063

Open
webdeveric opened this issue Jun 14, 2022 · 9 comments
Open

Check spelling of filenames #3063

webdeveric opened this issue Jun 14, 2022 · 9 comments

Comments

@webdeveric
Copy link

Is your feature request related to a problem? Please describe.

It would be nice to have an additional cli flag or command that spell checks the filename.

Describe the solution you'd like

Create an additional flag in lint (something like --check-filenames).
If it is true then whenever the file is being checked, it should also spell check the filename and report any issues.

Describe alternatives you've considered

Maybe this should be its own command instead of a lint flag?

Additional context

NA

@Jason3S
Copy link
Collaborator

Jason3S commented Jun 14, 2022

@webdeveric,

Help me understand your use case. What are you trying to do by spell checking file names?

Please note, this can already be done using stdin and a command like ls:

ls -1a | cspell stdin

@webdeveric
Copy link
Author

webdeveric commented Jun 14, 2022

@Jason3S In some projects at work, I've noticed some filenames are misspelled and I'd like a way to auto detect them in a reliable manner. We're using lint-staged too so having a flag I can set when calling cspell in that context would be nice.

I'm using stdin today and it works well enough, but it could be better. The output is not very useful. Something showing the full path (or relative from cwd) to the file and what is wrong would be helpful.

When trying to get a list of all files in a directory, including sub-folders, sometimes ls -R1a takes a while and may include folders that we don't need checked, like node_modules.

@Jason3S
Copy link
Collaborator

Jason3S commented Jun 14, 2022

@webdeveric,

There are a few workarounds:

git diff --name-only --cached | cspell stdin

or

lint-staged.config.cjs

export default {
  '*': (absolutePaths) => {
    return `echo "${absolutePaths.join('\n')}" | cspell stdin`
  },
}

@webdeveric
Copy link
Author

@Jason3S The first suggestion does detect changed files, but the output is not very helpful.

To illustrate this, I intentionally created two files with spelling issues (one in my project root and the other in a deeper sub folder), staged them in git, then ran git diff --name-only --cached | npx --no cspell -- stdin.

1/1 ./stdin 349.37ms X
/:1:5 - Unknown word (spling)
/:2:22 - Unknown word (speling)
CSpell: Files checked: 1, Issues found: 2 in 1 files

This output does not tell me what file needs to be fixed.

The second suggestion just doesn't work. I was trying that method this morning and lint-staged never failed due to spelling. It would always pass when the exact same command fails when I run it manually.

@webdeveric
Copy link
Author

I found a workaround for the lint-staged config. The command needs to be wrapped in sh -c ''.

Example:

export default {
  '*': (absolutePaths) => {
    return `sh -c 'echo "${absolutePaths.join('\n')}" | cspell stdin'`
  },
}

@Jason3S
Copy link
Collaborator

Jason3S commented Jun 14, 2022

Please try:
git diff --name-only --cached | npx cspell --no-progress --show-context stdin

image

The --no seems to mess things up.

@webdeveric
Copy link
Author

webdeveric commented Jun 14, 2022

To get around the --no issue, you have to use -- after cspell like so:

git diff --name-only --cached | npx --no cspell -- --show-context stdin

The --show-context option does help with the output 👍, but I think having this functionality built in and not relying on these work-arounds would be nice.

Working lint-staged.config.mjs example:

import { relative } from 'node:path';
import { cwd } from 'node:process';

/**
 * @type {(filenames: string[]) => string[]>}
 */
const relativeFilenames = (filenames) => {
  const root = cwd();

  return filenames.map((file) => relative(root, file));
};

/**
 * @type {Record<string, (filenames: string[]) => string | string[] | Promise<string | string[]>}
 */
export default {
  '*.{js,ts,tsx}': (filenames) => `eslint --fix ${relativeFilenames(filenames).join(' ')}`,
  '*.{js,ts,tsx,json,md,css,yml,yaml}': (filenames) => {
    const files = relativeFilenames(filenames);

    return [
      `cspell lint --no-progress --no-summary --no-must-find-files ${files.join(' ')}`,
      `sh -c 'echo "${files.join('\n')}" | cspell --show-context stdin'`,
      `prettier --write ${files.join(' ')}`,
    ];
  },
};

@Jason3S
Copy link
Collaborator

Jason3S commented Jun 14, 2022

@webdeveric,

Thank you for the working example.

I'm ok with an option to spell check the file name. But a few things need to be worked out:

What should be checked

  • Should it check the full path and file name including extension?
  • Should it check only the relative path from the cwd?
  • Should extensions be checked?
  • How to determine what is an extension?
  • Should only the filename + ext be checked (this might be the best option).

When to check

  • before/after the file is checked
  • using a separate command

How to report

  • How to emit issues? The spell checker tries to emit well defined errors that can be clicked on in supported terminals.
  • Where does something like Provide SARIF Output report ? #2083 apply?

@webdeveric
Copy link
Author

Here's my 2 cents:

What should be checked

I think it would be nice if the relative path to the file from the cwd were to be checked.
Example: Split the path on the separator (platform specific) then checking each part.

I've used https://nodejs.org/api/path.html#pathextnamepath to get the extension, but that doesn't account for common multi-extensions, like .tar.gz. I've used a RegExp pattern to allow users to customize this functionality in one of my projects. Something like this could work: https://github.com/webdeveric/webpack-assets-manifest/blob/206edd4d90cec9249d6617f1c48c9e79471c010e/src/WebpackAssetsManifest.js#L197-L212

I don't think extensions should be checked, unless there is a dictionary of all file extensions.

When to check

If the functionality is going to be added to the lint command, I think after checking the file would be fine, but I don't know if it matters much. It might if the options are set to fail fast.

Some people might not need or care if their file are spelled correctly so having it as a separate command would be OK too. I'd go with whatever is easiest and most efficient.

How to report

I've not been able to click on output from cspell so perhaps I should try a different terminal 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants