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

Allow capital letters in custom lessons #107

Closed
paulfioravanti opened this issue Feb 1, 2023 · 7 comments · Fixed by #108
Closed

Allow capital letters in custom lessons #107

paulfioravanti opened this issue Feb 1, 2023 · 7 comments · Fixed by #108

Comments

@paulfioravanti
Copy link
Contributor

When reading in word lists for custom lessons, it seems that all words get lowercased.

String filterLetters(String text)
{
String lowercase = text.MakeLower();

For word lists that contain person or place names etc, it would great to be able to keep the original word case containing capital letters, perhaps in a config option(?), rather than have convert-to-lower be the default.

@paulfioravanti
Copy link
Contributor Author

In case anyone else sees this, for reference, I made this change in my fork here.

@paulfioravanti
Copy link
Contributor Author

@mmaulwurff was there a specific reason you can recall that necessitated downcasing all letters in custom word lists/lessons?

@mmaulwurff
Copy link
Owner

I don't remember why I did this. Possible reasons:

  • lesson difficulty: for faster typing.
  • performance: for shorter word lists.

@paulfioravanti
Copy link
Contributor Author

Would you consider either:

  • loosening the behavior so that there's no unexpected limitations on custom lessons (I say unexpected because I had to go look at the code to see why my word lists weren't being displayed as-is)?
  • allowing parameters like whether capital letters can be used or not, and min/max word length, to be configurable in the Typist UI menus?

If you think that these suggestions aren't appropriate for your project, that's okay! I'd be happy to just continue tweaking my fork where I can :)

@mmaulwurff
Copy link
Owner

I think removing all the limitations is the best idea. Letter case, allowed characters, word length - let it be decided by the text itself.

Now I remember - the idea behind custom lesson was to allow the user throw any text at Typist and it would just work. That idea proved to be not feasible, as some preparation and manual filtering was always needed.

@paulfioravanti
Copy link
Contributor Author

So, if I understand your comment, you'd be in favor of getting rid of the filterLetters(contents) and filterWords(words, result._wordSet) functions altogether, but that wasn't possible due to some filtering always being needed. In that case, would you support at least loosening the filters in some ways, like allowing capital letters and/or allowing short(er)/long(er) words? Or, are those conditions the ones that you mentioned you always needed?

If those specific filters need to be there for some reason, then I respect that, and I'm happy to close this issue. I'm planning to keep the altered conditions in my fork, though, but are there any landmines I should watch out for by keeping them in?

@mmaulwurff
Copy link
Owner

I may have not expressed myself clearly. I'd be in favor of getting rid of filter functions altogether, and I see no disadvantages of that.

Initially, I wanted to accept all kinds of text - like, full stories. Hence the filters - to extract words from the text. But then I found out that the best way to use custom lessons is with simple word lists.

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 a pull request may close this issue.

2 participants