rust-lang / ferris-says

A Rust flavored implementation of `cowsay`
https://crates.io/crates/ferris-says
Apache License 2.0
194 stars 34 forks source link

Unclear documentation for `max_width` #30

Open jeroen1602 opened 2 years ago

jeroen1602 commented 2 years ago

I was going through the rust startup documentation trying to learn the language and started messing around with the example hello world code.

If you use non ASCII characters then they may take up more space than just a single byte (in UTF-8).

The code documentation for the say method's max_width field says :

max_width is the maximum width of a line of text before it is wrapped

Which would imply that the code will wrap on the character and not on the number of bytes since the word text is used.

For example the following code:

fn main() {
    let stdout = stdout();
    let message = "🐢🐢🐢";
    // Three characters so the width should be 3
    let width = 3;

    let mut writer = BufWriter::new(stdout.lock());
    ferris_says::say(message.as_bytes(), width, &mut writer).unwrap();
}

produces:

 ____
/ 🐢 \
| 🐢 |
\ 🐢 /
 ----
        \
         \
            _~^~^~_
        \) /  o o  \ (/
          '_   -   _'
          / '-----' \

While the expected result (to me) was:

 ________
< 🐢🐢🐢 >
 --------
        \
         \
            _~^~^~_
        \) /  o o  \ (/
          '_   -   _'
          / '-----' \

My proposal is to either change the code so that it would cut at the character instead of at the byte and/ or update the documentation.

Though then again this is a package just for fun so it isn't the end of the world if nothing changes.

mgattozzi commented 1 year ago

I think you're right in that it should be 3 "characters" how we as people would see them, not bytes. I think this was just an oversight. I mean I wrote a lot of this stuff years ago when I wasn't as familiar with things like utf-8 and text processing. Hell I'd update or throw away most of the code here today and rewrite it to be more idiomatic.

valters commented 1 year ago

Would it be possible to keep the old signature around (mark it deprecated, etc), so that existing tutorials and code examples don't break right away. (I mean, tutorials like https://www.rust-lang.org/learn/get-started ) Since this package is something one just starting out with Rust will encounter first, it would be great to show proper patterns how to evolve APIs.