Limiting size of osc_raw in no-std mode

[thejpster]

I am writing an OS and I was in need of an ANSI parser. I started to write my own, and then was pointed at your excellent crate. Thank you!

I note that const MAX_OSC_RAW: usize = 1024 and that is a little large for me. Would you accept a PR to make it a const-generic? Something like:

/// Parser for raw _VTE_ protocol which delegates actions to a [`Perform`]
///
/// [`Perform`]: trait.Perform.html
#[derive(Default)]
pub struct Parser<const OSC_RAW_SIZE: usize = MAX_OSC_RAW> {
    state: State,
    intermediates: [u8; MAX_INTERMEDIATES],
    intermediate_idx: usize,
    params: Params,
    param: u16,
    #[cfg(feature = "no_std")]
    osc_raw: ArrayVec<u8, OSC_RAW_SIZE>,
    #[cfg(not(feature = "no_std"))]
    osc_raw: Vec<u8>,
    osc_params: [(usize, usize); MAX_OSC_PARAMS],
    osc_num_params: usize,
    ignoring: bool,
    utf8_parser: utf8::Parser,
}

The const-generic parameter will be there even in no-std mode, but as it has a default, I don't think it'll matter?

[chrisduerr]

Yeah this seems like a good use for a const generic. With no-std this is just dead code so it shouldn't have any effect other than requiring an annotation to silence the warning.

[thejpster]

Hmm. I tried this out and it doesn't work.

It will accept:

let p: Parser = Parser::new();

But it won't accept:

let p = Parser::new();

cannot infer the value of the const parameter `OSC_RAW_SIZE` declared on the struct `Parser`

I could kludge it so the no_std case has to do a little bit of extra work, but it seems the no_std feature flag is actually on by default, so it would affect almost everyone and be a very breaking change.

I'll have a think to see if there's a way to do it without causing issues. If not, I'll just have to fork it :/

[chrisduerr]

I'm not sure right now, but that might be due to the definition of the new function or its impl block. I'm pretty sure it should be possible, considering this works just fine:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=4501d8b7f2eaba38c2e9301f7d91eaa9

[thejpster]

But this doesn't....

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ec2b871c6b44c72a7a2f5d053c0a61c5

[chrisduerr]

This works if you want to keep the API consistent:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e3ffb6d1a0b86b3948fa93b3b5db725b

[thejpster]

But now you're duplicating everything....

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=faa065d418df01dabb4504af908dba28

[thejpster]

Oh, OK maybe this does work...

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b21f23f38c3f663142dacf5fe313c85b

[thejpster]

Although again I'll note, you have no_std turned on by default so almost everyone is using the ArrayVec version which needs a const-generic. You might want to flip that so it is off by default, but that'll be a breaking change (for anyone actually using it on a no-std target).

[chrisduerr]

Although again I'll note, you have no_std turned on by default so almost everyone is using the ArrayVec version which needs a const-generic.

This isn't really accurate. "Almost everyone" is Alacritty and that has no_std turn off.

You might want to flip that so it is off by default, but that'll be a breaking change (for anyone actually using it on a no-std target).

I see no reason to make this breaking change.

[thejpster]

This isn't really accurate. "Almost everyone" is Alacritty and that has no_std turn off.

Of course, my mistake. I will try and submit a PR that allows Alacritty to build unchanged but also allows no-std builds to set how much RAM to use.

Edit:

https://crates.io/crates/strip-ansi-escapes appears to be your most-download dependent on crates.io, because I think it's used by cargo. But they also turn off default features and so operate in std mode.