search

Aloso / to-html

Utilities for making the colo documentation
MIT License
72 stars 10 forks source link

Do not panic on invalid format of ANSI string #10

Closed DmitryAstafyev closed 2 years ago

DmitryAstafyev commented 2 years ago

Thanks for this crate!

I think it isn't good to panic in case of an invalid format of ANSI string. Parser (or convertor) should just return a suitable error in such kinds of cases.

DmitryAstafyev commented 2 years ago

@Aloso Hello, could you take a look on it?

Aloso commented 2 years ago

Thank you for your pull request! At first sight the changes look good.

I'm quite busy today, but I'll look at it more thoroughly this evening.

Aloso commented 2 years ago

I'll release a new version now. Thanks for fixing this bug and the clippy warnings!

Aloso commented 2 years ago

After re-reading the code, I don't think it could actually panic. parse_4bit and parse_4bit_bright are used here:

    match code {
        //...
        30..=37 => Ansi::ForgroundColor(Color::parse_4bit(code - 30)?),
        //...
        40..=47 => Ansi::BackgroundColor(Color::parse_4bit(code - 40)?),
        //...
        90..=97 => Ansi::ForgroundColor(Color::parse_4bit_bright(code - 90)?),
        100..=107 => Ansi::BackgroundColor(Color::parse_4bit_bright(code - 100)?),
        _ => {
            return Err(Error::InvalidAnsi {
                msg: format!("Unexpected code {}", code),
            })
        }
    }

So it's guaranteed that the value passed to these functions is between 0 and 7.

DmitryAstafyev commented 2 years ago

you are absolutely right, calling these methods from ansi/iter_next give a guarantee... hmm I've just rollback my fork to commit 55ccdf5e9827c1d84e9e37fee70c993d05d1532a and I'm not able to reproduce a bug, which I had with panic on unreachable. Hmm weird. BTW I'm using your crate with web assembly.

In any way, thanks for the changes. Crate works fine and stable

DmitryAstafyev commented 2 years ago

I think I know what it was. On crate.io it says for version 0.1.0 the last update was "almost 2 years ago" (13.11.2020). But the last commit 55ccdf5e9827c1d84e9e37fee70c993d05d1532a (excluding today's changes) was 13.12.2021. Well, I think the reason why I have gotten a bug, was outdated metadata on crate.io. But again, in any way thanks for updates.