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

Ejf/improved spellcheck 4319 4320 #4330

Merged

Conversation

eli88fine
Copy link
Contributor

@eli88fine eli88fine commented Apr 9, 2021

Description

Skips spellchecking on code flanked in single or double backticks in comments/docstrings. Skips spellchecking on Python directives if and only if they are directly at the beginning of a comment (for tools such as black, flake8, zimports, isort, mypy, bandit, pycharm)

Type of Changes

Type
🐛 Spellchecker was overzealous about python code and tool directives in comments

Also some minor correcting to spelling errors in the code of spelling.py itself and minor refactor to remove some duplicate code (added the RegExFilter parent class).

Also, corrected a typo in the Changlog for 2.7.0 in an entry I submitted.

Related Issue

Closes #4319, #4320

@eli88fine
Copy link
Contributor Author

@Pierre-Sassoulas - please let me know how this looks

@coveralls
Copy link

coveralls commented Apr 9, 2021

Coverage Status

Coverage decreased (-0.03%) to 91.579% when pulling d75cb29 on eli88fine:EJF/improved-spellcheck-4319-4320 into 5e1928b on PyCQA:master.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, I made a small comment let me know what you think. Also some lines are a little long, (ie. horizontal bar in github review) could you cut them to something more reasonable please?

pylint/checkers/spelling.py Show resolved Hide resolved
pylint/checkers/spelling.py Show resolved Hide resolved
pylint/checkers/spelling.py Outdated Show resolved Hide resolved
@@ -308,9 +333,25 @@ def _check_spelling(self, msgid, line, line_num):
initial_space = 0
if line.strip().startswith("#") and "docstring" not in msgid:
line = line.strip()[1:]
# A ``Filter`` cannot determine if the directive is at the beginning of a line, nor determine if a colon is present or not (``pyenchant`` strips trailing colons). So implementing this here.
for iter_directive in (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be an option in the pylintrc, what do you think ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the tuple of directives to ignore? If so, sure...but I've never worked with obtaining options from pylintrc before---is there a good spot in the codebase to look to get oriented to that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. it's just the .config. Yep, I think I can do that. I'll have it default to this pre-existing list if none is present in pylintrc if that's OK.

@Pierre-Sassoulas Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label Apr 9, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.7.5 milestone Apr 9, 2021
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Pierre-Sassoulas Pierre-Sassoulas merged commit 316fc0d into pylint-dev:master Apr 10, 2021
@Pierre-Sassoulas
Copy link
Member

Thank you for implementing this check :) !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have spellchecker ignore code in docstring or comments (i.e. things flanked with double backticks)
3 participants