Reading non-ASCII characters encoded as UTF-8 doesn’t work well

[gchp]

Reading non-ASCII characters encoded as UTF-8 doesn’t work well

[gchp]

Yeah, if you open view.rs too you'll see there's a funny character in there → which gets displayed all wrong. Something needs to be added which will do some checking on special characters like this before drawing them with rustbox. I haven't spent a whole lot of time investigating Ascii characters with Rust just yet, so I'm not sure what exactly I need to do to make this work. If anyone has any pointers I'd love to hear them!

[ftxqxd]

I experimented a little and made a short patch that fixed this. I guess I should submit a PR? Edit: It’s actually looking a little trickier than I thought. I wonder if making Line store a String instead of a Vec<u8> would be a reasonable solution?

[gchp]

Yep, PRs are more than welcome!

I actually used to have Line store a String though I moved away from it as I thought Vec<u8> was more sensible and more efficient. I'm open to being convinced otherwise, though. What benefits are you seeing with using String instead of the vec?

[ftxqxd]

String enforces that its contents are valid UTF-8, and is designed to work with characters rather than bytes. This is both a good and a bad thing: it’s good because you get methods for working with characters instead of bytes, which is what you normally want when showing/editing text, but it’s bad because if you get something that isn’t valid UTF-8, you’re going to have a hard time using something that cannot be safely constructed or used when it contains invalid UTF-8. Rendering invalid characters on a terminal doesn’t really make any sense anyway, but it could be useful to show some kind of replacement instead, and using String might somehow complicate that. I personally think it’s fine to start with only basic UTF-8 support, and then later (if it has the demand) move on to support for invalid UTF-8 and even other encodings.

String shouldn’t be any less efficient as far as I know: it’s internally represented as a Vec<u8>. I guess some things like character indexing and character length are O(n) instead of O(1), but I personally think that correctness is more important than a slight speed boost in this case, and the performance shouldn’t be affected too much since it’s on a per-line basis (and lines aren’t usually that long).

[gchp]

Ok, these are some good points. This is definitely worth looking into. Shouldn't be a huge task in converting to use String. I'll give it a go and see if it solves our problem.