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

Pangram README claims input will only have ASCII #801

Closed
shaleh opened this issue Aug 15, 2017 · 5 comments
Closed

Pangram README claims input will only have ASCII #801

shaleh opened this issue Aug 15, 2017 · 5 comments

Comments

@shaleh
Copy link

shaleh commented Aug 15, 2017

The README says:

The alphabet used consists of ASCII letters `a` to `z`, inclusive, and is case
insensitive. Input will not contain non-ASCII symbols.

There are German characters in one test which causes failures if not ignored or counted. But even then tests fail if code does not handle characters outside of ASCII. Specifically, Russian.

--- FAIL: TestPangram (0.00s)
pangram_test.go:37: Pangram test [Широкая электрификация южных губерний даст мощный толчок подъёму сельского хозяйства.], expected [false], actual [true]
pangram_test.go:39: [Широкая электрификация южных губерний даст мощный толчок подъёму сельского хозяйства.] should not be a pangram because : Panagram in alphabet other than ASCII

I am OK with having to check if a rune is ASCII or not. But the README explicitly says not to worry about it. Or is 'symbols' here meant to mean things other than letters like braces, emoji, and the like? Either way the README is at best not clear and at worst wrong.

@tleen
Copy link
Member

tleen commented Aug 15, 2017

Thanks @shaleh, you are exactly right. That line does not make a lot of sense when you look at the test input. Should be fixed in #802.

Thanks for letting us know!

@tleen
Copy link
Member

tleen commented Aug 15, 2017

Going to keep the README but remove the non-ASCII compatible test entries.

tleen added a commit that referenced this issue Aug 15, 2017
This brings the tests into consistency with the README.

Resolves #801, also see #802
@shaleh
Copy link
Author

shaleh commented Aug 15, 2017

@tleen, I have not made it through all of the exercises yet. It would be good to have some of them show the capabilities of modern languages to in handling global inputs. I did not mind the Russian or German it was a good demonstration of not assuming English text. How many projects have we seen with the changelog entry "oh yeah, this ought to work outside of the US"?

@tleen
Copy link
Member

tleen commented Aug 15, 2017

I agree, especially considering how go really works the rune concept over characters as they are known in any other language. It is one of the pain points and great strengths of the language.

The way we addressed this specific issue is more or less to keep it consistent with the way this exercise is implemented across all all the Exercism tracks. It was decided that ASCII-only was the way to go here. We were an outlier. It is still a fairly early exercise in most tracks so that makes sense not to bring up character set issues just yet.

Don't worry there are definitely exercises coming up with non-ASCII stuff. Runetime!

@shaleh
Copy link
Author

shaleh commented Aug 15, 2017

Make sense. Sounds good.

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

No branches or pull requests

2 participants