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

Support inline ignores #1212

Closed
whoopsmith opened this issue Aug 2, 2019 · 31 comments · Fixed by #2400
Closed

Support inline ignores #1212

whoopsmith opened this issue Aug 2, 2019 · 31 comments · Fixed by #2400

Comments

@whoopsmith
Copy link

I'd like to use codespell as part of an automated check workflow. For that I need to be able to ignore certain cases on a per instance bases.

Here's an example. In the code base there is a command called 'Trim All'. The comment contains a statement like:
// Track how many Trim Alls have been issued.

codespell finds Alls as a possible error.

I don't want to add 'Alls' to the ignore words list as it could be a real error in other cases. But I don't want this message generated on every run either.
The ignore word list only supports single words So I can't do 'Trim Alls' as an ignore.

Some sort of in line keystring that allows the next occurrence of a specific word error would be useful. Most code formatters, pretty-printers, and static analysis tools have some sort of inline // disable-whatever option that allows you to skip the trigger on a per instance basis.

@jolivier23
Copy link

Or something like pylint does where you say
/* codespell: ignore <expression|list of words> */ with a matching "find" or "unignore" comment

@eavanvalkenburg
Copy link

This is a must when using codespell with a large multi-dev project, because you don't want to then alter the whitelist for every individual thing and what is correct in some contexts is not correct in others, so similar to pylint, flake8, etc adding a inline comment that tells codespell to ignore that particular line would be very welcome!

@vadz
Copy link

vadz commented Jun 27, 2020

What about extending the ignore file format to have a second column restricting the entry to being used for the given file or, better, giving line or range of lines in the given file? Sometimes you can't modify the sources themselves, but you can always add a file with codespell exceptions.

@eavanvalkenburg
Copy link

In my case I don't want to change some project level thing for my one file that needs an exclusion and where in some contexts I need the exception, while in other contexts it doesn's and should be flagged, so just the single line to be ignored would be far the easiest then, my case is with Home Assistant, where all the integrations are created independently of the main but there is a pre-commit hook with codespell, that shouldn't be updated by individual developers working on single things, but mine failed because I had a field coming from a external service that I had to check against which was flagged by codespell, I ended up having to move the checking list to a seperate json file (which were excluded by the project level codespell config) but it made my code more complex (reading a external file, instead of just a constant in a dict).

@peternewman
Copy link
Collaborator

The --exclude-file/-x option already has the ability to list lines to ignore (although in a separate file, rather than decorating the line).

For example:
https://github.com/OpenLightingProject/ola/blob/master/.codespellignorelines

@vadz
Copy link

vadz commented Jul 6, 2020

Thanks for the -x option hint! I've totally missed it and it's perfect for my needs.

@odashi
Copy link

odashi commented Apr 19, 2021

Any updates on this?

@peternewman
Copy link
Collaborator

Any updates on this?

If --exclude-file/-x doesn't suit your usage @odashi , then someone still needs to propose a format (either a comment at the end of a line), or a comment on the line before, either with one syntax that works on multiple programming languages, or more likely a variable prefix (e.g. # or // or whatever and then a static match phrase).

Then someone (not necessarily the same person) will need to implement it.

Suggestions and pull requests welcome.

@ssbarnea
Copy link

Maybe someone can tell me how to avoid codespell yelling about that python line KEGEX = r"^st(?:dout)?$" -- dout being the problem. Am I supposed to skip entire file because of this? Sounds like using a BFG to kill a moskito.

@abhinavsingh
Copy link

abhinavsingh commented Jan 26, 2022

I have a test with following chunked encoded packet. It thinks nd is not spelled correctly. Should I ignore the entire test file?

b'HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n5\r\nHello\r\n5\r\n Worl\r\n1\r\nd\r\n0\r\n\r\n',

Reference: https://github.com/abhinavsingh/proxy.py/pull/1064/files#diff-fc90f112bb0886415d939834b630f798e633c63c977a60a69b60b8fd7fcecd42R94

@abhinavsingh
Copy link

I have a test with following chunked encoded packet. It thinks nd is not spelled correctly. Should I ignore the entire test file?

b'HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n5\r\nHello\r\n5\r\n Worl\r\n1\r\nd\r\n0\r\n\r\n',

Reference: https://github.com/abhinavsingh/proxy.py/pull/1064/files#diff-fc90f112bb0886415d939834b630f798e633c63c977a60a69b60b8fd7fcecd42R94

Quickest hack is to add following at top of the file:

    .. spelling::

       nd

@peternewman
Copy link
Collaborator

So @ssbarnea and @abhinavsingh (and others on this thread), you've currently got a few options, in rough order from simplest, but biggest impact on finding other typos through the most complicated but restricted.

  1. Skip the entire file with --skip
  2. Skip the relevant line(s) with --exclude-file
  3. Skip the specific typo with --ignore-words-list (e.g. dout or nd)
  4. Hide the typo with --ignore-regex (possibly better for @abhinavsingh case)

Quickest hack is to add following at top of the file:

    .. spelling::

       nd

I'm not aware of that syntax, is it specific to Jupyter notebooks?

@abhinavsingh
Copy link

@peternewman No I should have came back to fix that. I mixed it with sphinx spell check and added a directive for that instead 🙆‍♂️. Finally, I had to add the exclude directives for all .ipynb

See https://github.com/abhinavsingh/proxy.py/blob/develop/.pre-commit-config.yaml#L128-L133

@abhinavsingh
Copy link

4. Hide the typo with --ignore-regex (possibly better for @abhinavsingh case)

This will be much better. Will likely fix it in coming days.

@peternewman
Copy link
Collaborator

@peternewman No I should have came back to fix that. I mixed it with sphinx spell check and added a directive for that instead

Ah that makes sense.

This will be much better. Will likely fix it in coming days.

I'm glad that's an improvement for you.

@relsqui
Copy link

relsqui commented Mar 5, 2022

I'd still like to have inline waivers. I have one single place in one single file where I legitimately want to use the string "ERRO". I don't want to skip the file, because it might have other problems in it later. I don't want to allowlist that word, because if it appears anywhere else it's probably a mistake. I just want codespell to trust me that I know what I'm doing in this one particular case.

@rob-miller
Copy link

I think I need inline waivers because I have contributed code to a large open source project, and am blocked from committing documentation for it because codespell thinks my attribute .aks should be "ask". It seems to me that defining how the pre-commit hook works is a role for the project maintainers, rather than every developer, and my spelling quirk should not necessarily be applied across the entie project - or even my entire file as I may make typos as well.

How about

* <b>codespell:ignore [<n>]

That is, a line possibly containing other text (e.g. comment characters for whatever language), followed by at least one whitespace, followed by the string codespell:ignore to cause the current line to be ignored. The optional integer parameter would specify the number of succeeding lines to also ignore spelling on.

Additionally one could envision

* codespell:ignore <string>[, <string> ...]

As a way to specify strings to ignore in the current file from this line on.

@nrbnlulu
Copy link

very needed feature

kaste added a commit to kaste/codespell that referenced this issue Jul 11, 2022
Fixes codespell-project#1212

Following @rob-miller's suggestion implement inline ignore hints.

E.g.

```
crate  // codespell:ignore crate
abandonned abondon abilty  # codespell:ignore abondon,abilty
abandonned abondon abilty  # codespell:ignore  # to ignore all
```
kaste added a commit to kaste/codespell that referenced this issue Jul 11, 2022
Fixes codespell-project#1212

Following @rob-miller's suggestion implement inline ignore hints.

E.g.

```
crate  // codespell:ignore crate
abandonned abondon abilty  # codespell:ignore abondon,abilty
abandonned abondon abilty  # codespell:ignore  # to ignore all
```
@kaste
Copy link
Contributor

kaste commented Jul 11, 2022

Why not give @rob-miller's idea an implementation #2400? Look at the tests https://github.com/codespell-project/codespell/pull/2400/files#diff-02a558d6c6bc59b5b9e5c7fc4d54893d875b4d7cceecfc151b9d729688bef061 Is this safe enough, too naive, too strict?

Supports . codespell:ignore someword,anotherword and wildcard . codespell:ignore. Ignoring the next line (or the next couple of lines) is out-of-scope for the first commit.

@ssbarnea
Copy link

Please make it compatible with cspell and we will all be happy, no need to implement all features supported by cspell. Check https://cspell.org/configuration/document-settings/ for ignore syntax, so we would not have to write two different sets of inline-ignores if we use both tools.

@kaste
Copy link
Contributor

kaste commented Jul 22, 2022

@ssbarnea Honestly, erm probably no. Using cspell: itself is unlikely as usually we don't advertise for other projects. It would be confusing even if we use one of their other prefixes as cspell has lots of options and I only implemented the "ignore on this line" which I don't even find the equivalent for cspell. So we don't support any of the options cspell offers but implement one thing cspell does not implement under the umbrella prefix cspell:. But maybe I didn't find it on their documentation.

@ssbarnea
Copy link

True that it makes little sense to use a namespace of another project but if i remember well cspell can also use a generic namespace, like spell, spelling, -- need to dig as I do not remember exactly. At this moment I reuse the whitelist between the two projects.

@debarshiray
Copy link

One problem that I am hitting with --ignore-regex is that the invocation of codespell with the incorrect spelling in my project's build files itself is getting flagged.

In Toolbx, I have this string complet in one of the files that I want to skip:

    if strings.Contains(command.Name(), "complet") {

Driven by paranoia, OCD and this absurd desire to avoid maintaining another auxiliary file for --exclude-file, I came up with:

--ignore-regex '^\t\tif strings\.Contains\(command\.Name\(\), "complet"\) {$'

I put in my meson.build and that's now getting triggered.

kaste added a commit to kaste/codespell that referenced this issue Feb 6, 2023
Fixes codespell-project#1212

Following @rob-miller's suggestion implement inline ignore hints.

E.g.

```
crate  // codespell:ignore crate
abandonned abondon abilty  # codespell:ignore abondon,abilty
abandonned abondon abilty  # codespell:ignore  # to ignore all
```
kaste added a commit to kaste/codespell that referenced this issue Feb 17, 2023
Fixes codespell-project#1212

Following @rob-miller's suggestion implement inline ignore hints.

E.g.

```
crate  // codespell:ignore crate
abandonned abondon abilty  # codespell:ignore abondon,abilty
abandonned abondon abilty  # codespell:ignore  # to ignore all
```
@epage
Copy link

epage commented Mar 9, 2023

@kaste looking at the link @ssbarnea gave

Using cspell: itself is unlikely as usually we don't advertise for other projects

Supported namespaces: cSpell, spell-checker, spellchecker, cspell

I only implemented the "ignore on this line" which I don't even find the equivalent for cspell.

"// cspell:disable-line – disables checking for the current line."

I'm the maintainer of typos and think it'd be a great idea if we could find common ground for these directives. I think supporting spellchecker:disable-line as a minimum is reasonable.

cspell's in doc settings parser is using regex unfortunately. I think it'd be nice to formalize what is supported / how.

Personally, the part that doesn't thrill me is having to track the comment rules per language as that is a large can of worms (which syntax, whether nesting is supported, being exhaustive enough, etc)..

@12rambau
Copy link
Contributor

12rambau commented Jul 27, 2023

I was reading this thread and it seems that not all people are convinced of the interest of such a feature so let me give a real life example:

in my lib I need to gather some data from USDA wich associate a US state to a 2 letter code. I thus store a boring dictionnary at the start of my module:

ASSET_CODES = {
    "Alabama": "AL", "Arkansas": "AR", "Arizona": "AZ", "California": "CA", "Colorado": "CO",
    "Connecticut": "CT", "Delaware": "DE", "Georgia": "GA", "Florida": "FL", "Iowa": "IA", "Idaho": "ID",
    "Illinois": "IL", "Indiana": "IN", "Kansas": "KS", "Kentucky": "KY", "Louisiana": "LA", "Massachusetts": "MA",
    "Maryland": "MD", "Maine": "ME", "Michigan": "MI", "Minnesota": "MN", "Missouri": "MO", "Mississippi": "MS",
    "Montana": "MT", "Nebraska": "NE", "New Hampshire": "NH", "New Jersey": "NJ", "New Mexico": "NM",
    "Nevada": "NV", "New York": "NY", "North Carolina": "NC", "North Dakota": "ND", "Ohio": "OH", "Oklahoma": "OK",
    "Oregon": "OR", "Pennsylvania": "PA", "Rhode Island": "RI", "South Carolina": "SC", "South Dakota": "SD",
    "Tennessee": "TN", "Texas": "TX", "Utah": "UT", "Vermont": "VT", "Virginia": "VA", "Washington": "WA",
    "West Virginia": "WV", "Wisconsin": "WI", "Wyoming": "WY",
}

When I run the code spell pre-commit, it falls on "ND" which is the code for Nevada and offers me to replace it with "AND" or "2ND". I have 0 guarantee that this dict will remain where it is so skipping a specific line number in the file is out of question. I don't want to add "ND" to the ignore dictionary because I still want this to be corrected elsewhere. I don't want to skip the whole file because there are many other things here.

In this situation it would make perfect sense to add # codespell:ignore at the end of the line and basta.

Note that for black I use # fmt: off ... # fmt: on which offers even more controls but that's for another issue ;-)

@chrisgrieser
Copy link

I came up with a simple idea to "emulate" inline code ignores:

codespell --ignore-regex=".*codespell-ignore$"

This will ignore any line that contains codespell-ignore at the end, effectively allowing for inline-comments like // codespell-ignore

@jdelStrother
Copy link

I came up with a simple idea to "emulate" inline code ignores:

codespell --ignore-regex=".*codespell-ignore$"

This will ignore any line that contains codespell-ignore at the end, effectively allowing for inline-comments like // codespell-ignore

Neat! Though, in case anyone else tries it, it took several minutes to run for me.
I narrowed the problem down to my tsconfig.tsbuildinfo file which contains a single 400kb line - after adding that to the skiplist it's behaving ok.

@jdelStrother
Copy link

Neat! Though, in case anyone else tries it, it took several minutes to run for me.
I narrowed the problem down to my tsconfig.tsbuildinfo file which contains a single 400kb line - after adding that to the skiplist it's behaving ok.

... following on from that, I've tweaked the regex to .{1024}|.*codespell-ignore.* so that it just auto-ignores any lines longer than 1024 characters without the catastrophic backtracking.

@bartlettroscoe
Copy link

For our project, I think we are going to write a driver script for codespell that runs codespell file-by-file and allows you to specify additional works to ignore on a file-by-file basis. Anyone else already do this? See:

I would hate to have to create and maintain a simple driver tool but it may be worth it to allow for file-by-file specific ignores.

kaste added a commit to kaste/codespell that referenced this issue Feb 8, 2024
Fixes codespell-project#1212

Following @rob-miller's suggestion implement inline ignore hints.

E.g.

```
crate  // codespell:ignore crate
abandonned abondon abilty  # codespell:ignore abondon,abilty
abandonned abondon abilty  # codespell:ignore  # to ignore all
```
@buhtz
Copy link

buhtz commented Feb 17, 2024

FYI: On reddit I opened a question if it might be possible to ignore the next line.

# codespell-ignore-next
print('Some missspellled texxT.')

@buhtz
Copy link

buhtz commented Mar 10, 2024

Dear @kaste I saw your commit 0f6636d in your own fork.

Did you opened a PR for this. On first look I would treat this as a high quality contribution.

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

Successfully merging a pull request may close this issue.