Skip to content

Add renaming advice based on Twitter engineering guidelines - #1607

Open
hadess wants to merge 7 commits into
codespell-project:mainfrom
hadess:wip/hadess/twitter-usage
Open

Add renaming advice based on Twitter engineering guidelines#1607
hadess wants to merge 7 commits into
codespell-project:mainfrom
hadess:wip/hadess/twitter-usage

Conversation

@hadess

Copy link
Copy Markdown
Contributor

As listed in:
http://web.archive.org/web/20200706172154/https://twitter.com/TwitterEng/status/1280188545743745024

Note that “placeholder” was not known in my dictionary so replaced with
“place-holder”, and that pronouns were not added, as adding renaming
advice based on those would not work very well with codespell.

Closes: #1597

@peternewman

Copy link
Copy Markdown
Collaborator

Note that “placeholder” was not known in my dictionary so replaced with
“place-holder”

What's your dictionary? It's shown as being in the core US one and both GB ones (let alone large). What does the CI say?

@peternewmanpeternewman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry I didn't really think this through when I saw the data, I just raised an issue.

We don't currently support a space in the typo, so for now everything apart from grandfathered needs to go sorry. Unless you want to hyphenate them for now? There seem to be a few hyphenated hits on grep.app for dummy, sanity and man.

@peternewmanpeternewman added the dictionary Changes to the dictionary label Jul 29, 2020
@hadess

Copy link
Copy Markdown
ContributorAuthor

Would we want to add support for spaces in the typo term? Or is that going to be a problem because the items won't be found in the dictionary?

@peternewman

Copy link
Copy Markdown
Collaborator

Would we want to add support for spaces in the typo term? Or is that going to be a problem because the items won't be found in the dictionary?

Probably yes in the long term (see e.g. #74 ), I think as part of that you'd then fix the dictionary checker to check each word rather than the whole correction.

It would need a bit of work though, as our regex currently finds words, you'd need to then take pairs or words and see if they match a correction, probably while skipping pairs with e.g. a comma or full stop. So I'd suggest let's get the basic one in first, and if you fancy having a go at the complicated case afterwards would be great.

@lurch

Copy link
Copy Markdown
Contributor

#1657 is relevant here 🙂

@hadess

Copy link
Copy Markdown
ContributorAuthor

I've rebased on top of #1657 for testing.


word_regex_def = u"[\\w\\-'’`]+"
# NOTE: flake8 suppression due to it not liking \] escape sequence
word_regex_def = u"([\\w\\-'’`]+)([.,?!-:;><@#$%^&*()_+=/\]\\[])?" # noqa W605

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe flake8 is correct, \] is not a valid escape sequence in Python.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dictionaryChanges to the dictionary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Possible additional data source for the usage dictionary

5 participants

@hadess@peternewman@lurch@DimitriPapadopoulos@jima80525