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 spellchecker script #338

Merged
merged 3 commits into from
Dec 11, 2016
Merged

Add spellchecker script #338

merged 3 commits into from
Dec 11, 2016

Conversation

JIghtuse
Copy link
Contributor

This patches add spellchecker script and integrates it into CI. See README.txt and comments in the beginning of a script for usage details.

Fixes #118

@carols10cents
Copy link
Member

So I'm trying this out, and I think I'm confused or doing something wrong :(

I decided to update the dictionary to get the latest valid words we use that are not spelling errors. So I removed the current dictionary (rm dictionary.txt), then ran the spellchecker script so that it would generate a new dictionary (./spellcheck.sh).

Then i did a git diff to see the changes to dictionary.txt-- mine had all the capitalized words sorted first :-/ I tried changing sort -u to sort -uf, but this removed a bunch of words since it collapses the uppercase and lowercase versions together, so for example it removed "BitAnd" whereas your file contains both "bitand" and "BitAnd". Did you happen to generate dictionary.txt with something other than the command in spellcheck.sh by any chance? I'd like to be able to consistently reproduce dictionary.txt so that diffs are easier to read. I mean, I can just change the file to what I'm seeing, but if it's something that differs per system and a few people run it and we end up changing it back and forth... I'd rather not do that :)

@JIghtuse
Copy link
Contributor Author

JIghtuse commented Dec 1, 2016

@carols10cents IIRC I used the same script. But it is possible that I added some words and forgot to sort it after that. I'll try to regenerate it now.

@JIghtuse
Copy link
Contributor Author

JIghtuse commented Dec 1, 2016

@carols10cents strange, it generates almost the same file for me. The only issue I spotted is that I removed sort words (length <= 3) from dictionary, but forgot to remove them from dictionary generator. Fixing it now.

Maybe, we have some weirdly differents sorts? It would be strange, but what else can be wrong. Here is mine:

    sort --version
    sort (GNU coreutils) 8.25

@JIghtuse
Copy link
Contributor Author

JIghtuse commented Dec 1, 2016

Short words should not appear in dictionary now.

You described perfectly valid assumptions, I agree that it should work like this. And it works for me. We should investigate how it behaves on different machines. I am on Fedora 25 now, and it is fine with sort here (8.25). Tried it in Debian Jessie VM - works too (sort 8.23).

@carols10cents
Copy link
Member

Oh geez. It looks like an LC_COLLATE/locale issue after some investigation. And OSX and/or OSX's sort and/or OSX's collation doesn't do the same thing as on linux :-/

I'm going to need a bit more time to investigate my options for fixing this, but at this point it seems like it's going to be a solution on my end, not yours. I'm just going to leave this open for a little bit to see if I have a way I can reproduce the dictionary. If I can't figure something out in a few days, I'll merge and deal.

@carols10cents carols10cents merged commit 0e19b6a into rust-lang:master Dec 11, 2016
@carols10cents
Copy link
Member

I decided I don't feel like figuring out LC_COLLATE; I just won't use aspell's interactive dictionary adding; nor will I regenerate dictionary.txt from scratch. I want this too much :)

Thank you so much for your work on this, you're going to save us from so many embarrassing typos and misspellings!!!!! ❤️ ❤️ ❤️ ❤️ ❤️

@JIghtuse
Copy link
Contributor Author

@carols10cents Yay, glad to hear that! Feel free to cc me if you will have any problems/suggestions for this spellchecker.

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