Skip to content

Add tests for aspell dictionaries different from 'en' - #1690

Merged
peternewman merged 26 commits into
codespell-project:masterfrom
sebweb3r:test_more_dicts
Nov 3, 2020
Merged

Add tests for aspell dictionaries different from 'en'#1690
peternewman merged 26 commits into
codespell-project:masterfrom
sebweb3r:test_more_dicts

Conversation

@sebweb3r

@sebweb3rsebweb3r commented Sep 23, 2020

Copy link
Copy Markdown
Contributor

This adds additional tests in the en_GB-ise dictionary.

I can add en_CA and en_AU too

Closes#1689

This was referenced Sep 23, 2020
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
@lurch

Copy link
Copy Markdown
Contributor

I wonder if this PR would allow you to pull in any other words from #1626 ?

@sebweb3r

Copy link
Copy Markdown
ContributorAuthor

Quick test said no. unfortunaly.

@lurch

lurch commented Sep 23, 2020

Copy link
Copy Markdown
Contributor

LOL, typo in your last comment 😆

EDIT: ... which isn't in dictionary.txt yet!

@peternewman

Copy link
Copy Markdown
Collaborator

EDIT: ... which isn't in dictionary.txt yet!

Well spotted and volunteered @lurch ! 😆

@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.

Looks good, a few improvements that can be done.

Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
@sebweb3r

Copy link
Copy Markdown
ContributorAuthor

You know, those typos are the reason, why I'm here :-D

@sebweb3r

sebweb3r commented Sep 23, 2020

Copy link
Copy Markdown
ContributorAuthor

@peternewman I need help. The github action and appveyor are failing, but travis and my debian are fine with the tests/commits. Any idea, why?

Edit: found it!

Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated

@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.

A few comments and thoughts

Comment threadcodespell_lib/tests/test_dictionary.py
Comment threadcodespell_lib/_codespell.py Outdated
Comment threadcodespell_lib/_codespell.py Outdated
Comment threadcodespell_lib/_codespell.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
@sebweb3rsebweb3r changed the title Add tests for aspell en_GB-iseAdd tests for aspell dictionaries different from 'en'Sep 24, 2020
@lurch

lurch commented Oct 2, 2020

Copy link
Copy Markdown
Contributor

Not that it really matters, but does the "Test: Change en_GB to en_GB-ise" commit-title have things the wrong way around?

@sebweb3r

Copy link
Copy Markdown
ContributorAuthor

should work now like expected

@sebweb3r

Copy link
Copy Markdown
ContributorAuthor

and @larsoner

@larsonerlarsoner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didn't follow any of the discussion so let me know if there is some remaining issue. But looking at the code this looks good to me. @peternewman feel free to merge if you're happy

@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.

A few comments

Comment threadcodespell_lib/_codespell.py Outdated
Comment threadcodespell_lib/_codespell.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated
Comment threadcodespell_lib/tests/test_dictionary.py Outdated

@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.

A few more comments

Comment threadcodespell_lib/_codespell.py Outdated
Comment threadcodespell_lib/_codespell.py
@sebweb3rsebweb3r mentioned this pull request Oct 27, 2020

@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.

LGTM

@peternewman
peternewman merged commit c77fc5a into codespell-project:masterNov 3, 2020
@sebweb3r
sebweb3r deleted the test_more_dicts branch November 3, 2020 13:33
Sign up for freeto 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.

Some enGB words cause bugs with aspell

4 participants

@sebweb3r@lurch@peternewman@larsoner