inconsistent handling of graphemes 👩🏽‍🌾🧟🧞‍♀️🦹‍♂️🧔🧛🏾‍♂️

[drahnr]

Describe the bug
In various places, the length used for offset calculation is used in bytes, which breaks Dow when multicharacter graphemes are used in literals.

To Reproduce
Currently there is a lack of unit tests to cover this issue sufficiently ( = none)

Expected behavior
Spans must be calculated based on graphemes (superset of chars) OR bytes consistently.

[drahnr]

Requires an external library https://crates.io/crates/unicode-segmentation since String does not handle that too well https://doc.rust-lang.org/std/string/struct.String.html#method.chars

CC @laysauchoa

[drahnr]

Since graphemes are not even displayed in vscode, I think it's safe to say that operations on unicode chars is sufficient.

[laysauchoa]

CC @drahnr

My vscode actually shows emojis and I see that it gets read as it was a misspelled word.

although, I am not sure if it is common to use emojis in documentation. ^^

[drahnr]

It's more common than you'd expect, and you can see the issue already above, it should be one ^ instead of 4 (and it's not even a grapheme)

[drahnr]

@laysauchoa this should not be the case anymore, is it?

[laysauchoa]

cc @drahnr

yes ^^, this is not the case anymore.

error: spellcheck(Hunspell)
   --> /home/tmhdev/Documents/cargo-spellcheck/src/suggestion.rs:23
    |
 23 | ..ation. This is a very logn sentence with some errors and emojis Search Resul 😋😋
    |                                                                          ^^^^^
    | - Result or Restful
    |
    |   Possible spelling mistake found.
    |

How did it ignore emoji now or do not recognize as an error? Just curious!

Although, would this issue still be relevant due to some misalignment when emojis are present?

error: spellcheck(Hunspell)
   --> /home/tmhdev/Documents/cargo-spellcheck/src/suggestion.rs:23
    |
 23 | ... This is a very logn 😋😋😋😋 sentence with some errors and emojis Search Resul 😋😋
    |                                                             ^^^^^^
    | - emoji
    |
    |   Possible spelling mistake found.
    |

[drahnr]

Would you like to look into the offset issue, I would have expected this to be ok with the 0.3.0 release at least.

Could you create a PR with an end2end style test showing the issue, so this is easily reproducable, no matter if you want to fix it or not, that'd be very helpful already!

[laysauchoa]

Hi @drahnr.

Do you mean the offset issue when the emojis are being used, such as the example above?

Do you mean a test such as those ones [0]?
[0] https://github.com/drahnr/cargo-spellcheck/blob/master/src/suggestion.rs#L699

If yes, then yes, I can try to add. If not, please give me more info, and I will check it.

[drahnr]

Hi @drahnr.

Do you mean the offset issue when the emojis are being used, such as the example above?

Do you mean a test such as those ones [0]?
[0] https://github.com/drahnr/cargo-spellcheck/blob/master/src/suggestion.rs#L699

If yes, then yes, I can try to add. If not, please give me more info, and I will check it.

Yes, that's what I meant :)

[drahnr]

The largest remaining issue here is that the display of graphemes might be individual smilies but also combined into a single character so the width / offset of ^^^^ might be wrong.
The resolution here would be a way to probe the terminal and resolve that, plus a grapheme library, but this smells like distant future.

[drahnr]

This is an edge case, and unless somebody actually uses graphemes AND a terminal that can display them, this won't be an issue. This is not on the agenda for any-time-soon™. Closing.