Custom colors support

[lu-zero]

Would be acceptable to expose directly or proxy owo_colors and store in Info the Color enum extended to have a Custom variant?

I started looking at it and it seems simple enough beside design decisions regarding re-export.

[pacak]

As in a single custom color or a custom color scheme? What's your use case?

[lu-zero]

A custom color scheme, the bright-colors felt a bit too bright so I started to look into having the scheme matching the one I see in other clis.

[pacak]

I see. And the dull-colors is not colorful enough?

I was thinking about adding this support myself in the next breaking release. I'll probably drop owo_colors dependency as well in favor of constructing ansi sequences by hand. The idea for API I have something like this:

There's a struct that contains ansi sequences for main items - one field per type: metavar, literal, etc. Info holds a static reference to that and you can pass your own. bpaf defines a few colorschemes somewhere inside, feature flags pick the default, but user can pass their own.

The idea is to have something low overhead / low API surface. Gimme a few days to experiment first.

[lu-zero]

Thank you :)

I was planning to simply take one function per field type and simply expose owo_colors since all in all it is simple enough and the functions signature would be probably something along the line of fn(&str) -> impl Display or such.

[lu-zero]

I might have some time during the next week if I can help.

[pacak]

I might have some time during the next week if I can help.

I appreciate the offer ❤️

So end of September I started experimenting with changing metadata representation, this went fairly well and mid October - around your ticket time I was thinking about replacing it and calling the result 0.10 but then decided to try changing the internal parsing logic a bit more - restriction that positional items must be parsed last is confusing. This went well too so I ended up rewriting all the things :)

User facing API stays the same - I'm planning to drop a thing or two and rename a bunch of types you shouldn't be using by name anyway.

At this point I have an implementation that is about 70% complete parsing wise - no blockers there, just filling in missing todo!()s and missing API bits. Errors are still lacking as well as anything --help, markdown or manpage related.

One potential area of contribution is probably this help/markdown/manpage related stuff. Current version takes Meta and renders it - this couples things more than I like it, new version uses visitor pattern.

Second area is to look at rendering for examples (parts of the documentation that show derive and combinatoric examples + the output). Current version went though a few iterations and I'd still like to have something less clunky to use. Can probably be a standalone crate, not sure.

Then there's something can be improved with shell completion testing. Currently they are not running in CI and I'd like to change that.

Said all this - I expect it would still take a month or two or more before 0.10 is ready.

Let me know if any of it interests you and I'll make some tickets with my ideas as a place to talk about things.

[lu-zero]

Wow, that's a lot to look at :)

Probably I can fill in some of the todo!() missing if they are self-contained, I was thinking of using bpaf for the next release of rav1e so I might see firsthand if there are other gaps feature-wise compared with what we currently have.

[pacak]

Feature wise - should be more or less parity, I don't see anything super unusual.

Things like this become enums

...
  /// Number of tile rows. Must be a power of 2. rav1e may override this based on video resolution.
  #[clap(long, value_parser, default_value_t = 0, help_heading = "THREADING")]
  pub tile_rows: usize,
  /// Number of tile columns. Must be a power of 2. rav1e may override this based on video resolution.
  #[clap(long, value_parser, default_value_t = 0, help_heading = "THREADING")]
  pub tile_cols: usize,
  /// Number of tiles. Tile-cols and tile-rows are overridden
  /// so that the video has at least this many tiles.
  #[clap(
    long,
    value_parser,
    conflicts_with = "tile_rows",
    conflicts_with = "tile_cols",
    help_heading = "THREADING"
  )]
  pub tiles: Option<usize>,
...

fn is_power_of_2(val: &usize) -> bool {
    todo!()
}

/// THREADING
enum Tiles {
    Grid { 
        /// Number of tile rows. Must be a power of 2. rav1e may override this based on video resolution.
        #[bpaf(fallback(0), guard(is_power_of_2, "Must be power of 2")]
        tile_rows: usize,
        /// Number of tile columns. Must be a power of 2. rav1e may override this based on video resolution.
        #[bpaf(fallback(0), guard(is_power_of_2, "Must be power of 2")]
        tile_cols: usize
    }
    Count {
        /// Number of tiles. Tile-cols and tile-rows are calculated
        /// so that the video has at least this many tiles.
        tiles: usize
    },
}

And the CliOptions gets separated into smaller chunks like in cargo-show-asm

Or this

...
  /// Quantizer (0-255), smaller values are higher quality [default: 100]
  #[clap(long, value_parser, help_heading = "ENCODE SETTINGS")]
  pub quantizer: Option<u8>,
...

Is really this with minor cosmetic differences (all "encode settings" things get their own structure)

...
...
    /// Quantizer (0-255), smaller values are higher quality
    #[bpaf(fallback(100), display_fallback))]
    pub quantizer: u8

[pacak]

Probably I can fill in some of the todo!() missing if they are self-contained

Hmm... Probably not very self contained yet :) But we'll see how far I can get today.

[lu-zero]

Thank you :)

[pacak]

Found one mostly self contained example, requires dealing with non-utf8 strings. This todo specifically.

bpaf/bpaf_core/src/split.rs

Line 300 in aec308e

Some(_) => todo!(),

It's a replacement for this:

bpaf/src/arg.rs

Line 117 in 068f514

pub(crate) fn split_os_argument(input: &std::ffi::OsStr) -> Option<(ArgType, String, Option<Arg>)> {

Strings in Rust must be valid utf8, filenames doesn't have to be - parsers must know how to deal with it. OsString in stdlib is limited to what you can do with it, the idea is to strip ascii prefix, parse the prefix, figure out if it is one of -afoo, -a=foo, --arg=foo then compose the result from parsing the prefix and non-utf8 suffix back into Arg. split_str_param can parse ascii prefix just fine. So.

1. strip utf8 prefix - done.
2. parse prefix with split_str_param.
3. figure out what belongs to name and what to the value - put everything into Arg::Named with a value.
4. Add tests covering all the scenarios scenarios.

Example inputs, capital letters are non-utf8.

• input: -afOO, split gives "-af" and "OO", parse gives short named a with value f and suffix OO, output - short named a with value fOO

• input -a=FOO - split gives -a= and FOO, parse will probably fail, expected result is still short named a with value FOO

Some old tests here https://github.com/pacak/bpaf/blob/master/src/tests.rs

Pull request can go into bpaf-core branch