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

Update dictionary with more misspellings and disabled names #1425

Merged
merged 1 commit into from
Mar 6, 2020
Merged

Conversation

qyearsley
Copy link
Contributor

This PR:

  • Disables a few more person names: Amin, Ang, Anny, Bae, Chang, Que, Sargent, Wen
  • Also disables Synopsys (company), thru (informal).
  • Adds a few misspellings I've seen: callabck, cleand, notificaiton, seprated, serailze.

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 from me.

codespell_lib/data/dictionary.txt Outdated Show resolved Hide resolved
codespell_lib/data/dictionary.txt Outdated Show resolved Hide resolved
@qyearsley
Copy link
Contributor Author

qyearsley commented Mar 3, 2020

Thanks for review -- I still think it might be a good idea to consider including the names -- and also note, the dictionary already contains "tim->time, Tim, disabled due to being a person's name".

But if you feel that it's better to remove all mention of names and include misspellings that are also people's names, I could remove the names from this PR.

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/data/dictionary.txt Outdated Show resolved Hide resolved
codespell_lib/data/dictionary.txt Outdated Show resolved Hide resolved
codespell_lib/data/dictionary.txt Outdated Show resolved Hide resolved
@peternewman
Copy link
Collaborator

But if you feel that it's better to remove all mention of names and include misspellings that are also people's names, I could remove the names from this PR.

I'm merely another user, but the pragmatic solution may be to split the uncontentious non-name changes into another PR so they can be merged, while a consensus is potentially reached over what's in a name.

Copy link
Contributor Author

@qyearsley qyearsley left a comment

Choose a reason for hiding this comment

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

Now removed names from this PR

codespell_lib/data/dictionary.txt Outdated Show resolved Hide resolved
@qyearsley
Copy link
Contributor Author

Updated The idea in #103 is interesting; it seems like in general we could trend towards one more complicated dictionary, or multiple dictionaries/ignore-lists with more functionality

As-is, for my particular use case, I'm going to have to keep my own ignore list, although ignoring common proper nouns seems like a reasonable think to potentially include in codespell itself.

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.

LGTM, just one comment.

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.

LGTM

@peternewman peternewman merged commit 9a2361d into codespell-project:master Mar 6, 2020
@peternewman
Copy link
Collaborator

Thanks @qyearsley . Please open a new one with the other more "controversial" changes. And we'll see where #1437 goes too.

@peternewman
Copy link
Collaborator

peternewman commented Apr 6, 2020

@qyearsley we now have a solution. If you can add the names in a new PR to https://github.com/codespell-project/codespell/blob/master/codespell_lib/data/dictionary_names.txt would be great!

@qyearsley
Copy link
Contributor Author

Great news, good to know, thanks :-)

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

Successfully merging this pull request may close these issues.

2 participants