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

Acronym: add underscore test case #1436

Merged
merged 1 commit into from Jan 9, 2019
Merged

Acronym: add underscore test case #1436

merged 1 commit into from Jan 9, 2019

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Jan 8, 2019

Many regular expression libraries have a way to detect word boundaries,
but their definition of word characters includes underscores.

\b and \w metacharacters were designed to detect programming language keywords,
and it is a common mistake to use them to match words in natural languages.

@rpottsoh
Copy link
Member

rpottsoh commented Jan 8, 2019

@link2xt thanks for opening the PR. The Travis CI build failed. Please check the details and correct the issue that it is reporting. Let me know if you have any questions.

@sshine
Copy link
Contributor

sshine commented Jan 8, 2019

Excellent!

Should the acronym of "Foo_Bar" be "FB" and not "F"? And should the acronym of "_Foo" be well-defined?

This does not appear to be a clarification of the exercise, but an extension of it. And it seems to test two different things. So it would warrant two test cases and a major version bump.

I am personally content with not having them, but I'm not seeing a lot of solutions to this exercise. I think your suggestion to teach that \w in regex includes _ is one that benefits many tracks.

And I think that if the acronym of "Foo-Bar" is "FB", you can make the case for _. Is that what you're doing?

@petertseng
Copy link
Member

\b and \w metacharacters were designed to detect programming language keywords,
and it is a common mistake to use them to match words in natural languages.

This has subtly pointed out that up to this point, all inputs have been written in a form that one might see in natural language. I think FOO_BAR is something I would not expect to see in natural language (but I could be convinced about _BAZ_ if we assume that someone is trying to emulate underlining a word). So FOO_BAR would be a departure from existing test cases for this reason. Be prepared to accept this departure.

The above comment must not be read as either variant of "I {support, oppose} adding FOO_BAR".

@link2xt
Copy link
Contributor Author

link2xt commented Jan 8, 2019

This has subtly pointed out that up to this point, all inputs have been written in a form that one might see in natural language.

I guess I will change this to look like markdown-emphasized word then.

Many regular expression libraries have a way to detect word boundaries,
but their definition of word characters includes underscores.

\b and \w metacharacters were designed to detect programming language keywords,
and it is a common mistake to use them to match words in natural languages.
@link2xt
Copy link
Contributor Author

link2xt commented Jan 8, 2019

@sshine

Should the acronym of "Foo_Bar" be "FB" and not "F"?

I guess it is better not to test it then, because it is ambiguous, just like "FooBar", which is also not tested. But _foo_ should not be treated different from *foo* simply because your language has regex library.

@sshine sshine merged commit cacf1f1 into exercism:master Jan 9, 2019
@link2xt link2xt deleted the acronym_underscore branch January 9, 2019 05:17
sshine pushed a commit to exercism/haskell that referenced this pull request Oct 10, 2019
add underscore test case

fix the text example to pass the newly added test case

exercism/problem-specifications#1436
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants