Mark individual incorrect characters

[bolphen]

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) .

[Sntx626]

I've pulled the repo and checked out the commit (c4a1980), however the compiled version does not highlight individual characters properly.

Namely: It will only higlight the first correct characters of a word green, not all correct characters.

This results in wirld having w green and irld red, even though w rld should be green and i should be red.

[Sntx626]

This results in cargo throwing the warning:

warning: function `ceil_char_boundary` is never used
   --> src/ui.rs:346:4
    |
346 | fn ceil_char_boundary(string: &str, index: usize) -> usize {
    |    ^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default

[bolphen]

I removed the unused code.

[bolphen]

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.

[Sntx626]

Fixes the warning, still up for discussion if we want that behaviour.

What do other think?

[bolphen]

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]

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

[Sntx626]

Looks good! Now it does exactly what the PR title says and works great.

Thanks for the contribution ^^

[glazari]

Hey there, author of #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 #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]

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]

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.