add new read_chars method, fix bug in read_char

[grahame]

having a read_chars method is convenient and more efficient.

the old read_char method had a bug due to re-use of the 'w' width
variable as a loop counter and so was broken for wide characters,
this patch fixes that.

[ sorry for all the spam, a bit of a newbie to contributing via github ]

[brson]

This appears reasonable. Thanks!

Honestly, I haven't pondered the code in depth (too much bit twiddling), but I would be more confident with some accompanying tests, particularly a regression test for the issue you are fixing. You could add them to stdtest::io, probably using a string_reader.

Beyond that I wonder why chars_from_buf gets so little reuse out of the str::* methods, since there are some there for parsing unicode.

[grahame]

The previous read_char method had roughly the same unicode bit fiddling code - I agree that it's odd it's not shared. I can look at tidying that up if you like? Perhaps just get the character width, then pass a vec::slice of bytes to a method in str:: that returns the char.

I'll write some tests - I'll have a go this evening after work.

[brson]

Don't feel like you need to factor that out as part of this patch, but it might be worth investigating in the future.

[grahame]

Sure. Just to check - should I cancel this pull request and resubmit a new one with the patch and the tests when that work is done? I'm not quite up to speed on this github workflow.

[brson]

No need to cancel this pull request. If you just push to the same branch the pull request will update to reflect the new code.

[kud1ing]

Would it make sense to extract the core of chars_from_buf() and call that from read_char() instead of read_chars()?

Maybe this is not a real efficiency-issue, but i have a strange gut feeling about allocating a vector of chars, when one only needs to return one char in read_char() ...

[brson]

Thanks for the tests. If you want to extract chars_from_buf to make read_chars more efficient then go for it.

[brson]

@Lenny222 Oops, didn't realize you were providing feedback to grahame, and not grahame himself pondering future improvements. Hope you don't mind that I merged this.