search

max-niederman / ttyper

Terminal-based typing test.
MIT License
1.05k stars 76 forks source link

Mark individual incorrect characters #92

Closed bolphen closed 11 months ago

bolphen commented 11 months ago

As suggested in #75, when mistype happens, only mark the wrong characters starting from the first wrong character to the end, instead of the entire word.

A byproduct of marking individual characters is that it also fixes some issues mentioned in #77 (for example, accented letters like ä or é being counted as two characters).

Likely will conflict with #90. I'm submitting a PR since someone has requested it. Feel free to close if there are better solutions (or maybe use this as a temporary fix for v1, as the code change is not significant) .

bolphen commented 11 months ago

Yep... Sorry my description was misleading.

That was actually by design cuz sometimes one might mistype with extra characters, like world with wworld. It's not possible to tell that the two w both correspond to the same w, so the rest of the word will be marked as wrong anyway. So IMO it's easier to just mark from the first wrong letter. (And for example keyhero.com does the same.)

If one really wants each individual character to be marked, probably #90 is the way to go.

bolphen commented 11 months ago

Well another reasoning would be that it encourages you to go back and fix the mistyping, since that's what one would do in real-life typing. And the change helps you to see just how far back you should go (which is what I find useful anyway).

Also it's less work :D

bolphen commented 11 months ago

@Sntx626 Just redid the thing, which should do exactly what you are asking now.

glazari commented 11 months ago

Hey there, author of https://github.com/max-niederman/ttyper/pull/90 here. Just want to add a bit to the discussion.

Seems like the behavior of the latest version of this PR exactly matches the one in https://github.com/max-niederman/ttyper/pull/90. And it does this with less lines of code, which is nice.

I guess in my PR I mixed this functionality with a small refactor. My motivation was that I believe UI code should be a very thin wrapper over representation. So I wanted to separate a bit the logic of checking which parts of the word are correct from the data types associated with Ratatui itself.

I believe that doing this separation has 2 benefits:

  1. Its easier to test, since we can test the word-part logic without messing with Styles.
  2. Its easier to swap out the front end UI code. One could imagine swapping the terminal crate, or making a native GUI app, or even using this crate as a wasm core for a web based app.

The first point I think is important because, if we do want to evolve this crate to the vision of being more like monkey-type, then I think we need more tests to allow us to do more aggressive changes with confidence.

Having said that, I am not married to the other PR at all. If @max-niederman prefers to merge this one since it is a smaller code change, I wouldn't mind.

Sntx626 commented 11 months ago

I think @glazari has a point, staying with the ratatui seperation would offer a better codebase in the future.

Since both solve the same problem I'd say we should go for #90, since less code isn't always better code.

I want to stress though that I'm happy with both implementations and think the final choice should be left to @max-niederman.

max-niederman commented 11 months ago

I've been considering this for a little white, and I agree that #90 is a better implementation for basically the same reasons as @glazari listed. I'll close this in favor of moving forward with #90.