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

Add --ignore-regex #1592

Merged
merged 7 commits into from
Aug 10, 2020
Merged

Add --ignore-regex #1592

merged 7 commits into from
Aug 10, 2020

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Jul 8, 2020

This was originally for issue #676, but the preference there is for a different approach. We're keeping it because it may still prove useful, e.g., a regex like (?:[a-fA-F0-9]{0,4}:){2,7}[a-fA-F0-9]{0,4})\]? could be used to ignore spelling mistakes in ipv6 addresses.

Mechanically, this erases the matching text before the word regex is applied.

jonmeow added 2 commits July 8, 2020 09:19
This is for issue #676, where typos are found in actually-okay URIs/emails. Because these are closer to names in context, this ignores them.

Mechanically, this erases the URI/email text before the word regex is applied.
Copy link
Collaborator

@peternewman peternewman left a comment

Choose a reason for hiding this comment

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

A few comments

codespell_lib/tests/test_basic.py Outdated Show resolved Hide resolved
codespell_lib/tests/test_basic.py Outdated Show resolved Hide resolved
codespell_lib/_codespell.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

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

Switched this to be just --ignore-regex per issue #676 discussion

codespell_lib/_codespell.py Outdated Show resolved Hide resolved
codespell_lib/tests/test_basic.py Outdated Show resolved Hide resolved
codespell_lib/tests/test_basic.py Outdated Show resolved Hide resolved
@jonmeow jonmeow changed the title Add --ignore-regex for URI/email handling. Add --ignore-regex Jul 24, 2020
@jonmeow jonmeow requested a review from peternewman July 24, 2020 21:13
Copy link
Collaborator

@peternewman peternewman left a comment

Choose a reason for hiding this comment

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

Thanks for the pivot, a few comments on it.

codespell_lib/_codespell.py Outdated Show resolved Hide resolved
codespell_lib/tests/test_basic.py Show resolved Hide resolved
codespell_lib/tests/test_basic.py Show resolved Hide resolved
Copy link
Collaborator

@peternewman peternewman left a comment

Choose a reason for hiding this comment

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

Two minor bits sorry.

help='regular expression which is used to find '
'patterns to ignore by treating as whitespace. '
'When writing regexes, consider ensuring there '
'is boundary non-word chars, e.g., '
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit sorry, grammar wise it should be "ensuring there are boundary..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

codespell_lib/tests/test_basic.py Show resolved Hide resolved
@peternewman
Copy link
Collaborator

@larsoner do you want to review this, or are you happy with mine?

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

@peternewman feel free to merge when you're happy

Copy link
Collaborator

@peternewman peternewman left a comment

Choose a reason for hiding this comment

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

Great, thanks for this @jonmeow

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

Successfully merging this pull request may close these issues.

3 participants