bpaf icon indicating copy to clipboard operation
bpaf copied to clipboard

Support deprecated options

Open bwidawsk opened this issue 2 years ago • 4 comments

It would be helpful to have a unified way to express an argument is going to be deprecated (and may not work) at some future point. Bonus points if it could re-use #[deprecated] attribute

bwidawsk avatar Jan 02 '24 18:01 bwidawsk

I'm not sure if there can be a single unified approach for deprecation, but I'm open to implementing necessary bits so you can resolve your deprecation needs on your own :)

Let's go over several possible scenarios:

  • an option is renamed and there's no plans to repurpose the old name. In this case I'd make old name into a hidden alias and just leave it working. Users can still access it, as a programmer I only need to deal with a value parsed by the old name.
  • a flag is removed and new behavior is instead a default. In this case I'd replace the parser into something that parses into () and prints a warnings as it does. This can be done with existing tools - map.
  • a flag/option is removed and replaced with combination of flags/options that parses into the same value. Error message can say something like --foo FOO is deprecated, use --bar BAR [--baz BAZ] instead and parser combinator can take two parsers and produce a single one that prints a warning if deprecated parser succeeds and produces a value in either case.
  • a flag/option is removed, there's a suggested combination but more input is needed. Error message can say something helpful and exit if deprecated parser succeeds.

What kind of deprecation do you have in your case?

pacak avatar Jan 02 '24 19:01 pacak

Thanks for taking the time to consider this.

It's kind of a mix of 1, and 3. The cmdline argument is replaced by an entry in a config file, but I suppose in your domain, it's mostly just 1.

To be more detailed, previously scales for various outputs were specified via commandline:

/// Scaling values for outputs
///
/// Example: In order to have eDP-1 scaled to 2.0 and DP-1 to 1.0:
///   eDP-1=2.0,DP-4=1.0
#[bpaf(argument::<OutputScales>, env("SBRY_SCALING"), help("DEPRECATED!!!"))]
pub(crate) scaling: Option<OutputScales>,

Now a ron config file serves the same purpose, but provides more information per output

SudburyPolicy(
    sloppy_focus: false,
    outputs: [(name: "eDP-1", resx: 4096, scale: 1.5)],
)

So in the case where the ron config has no entry for the given output, I can easily populate something with only a scale, but it gets messy in the case where the ron config has an entry, and a scale was specified on the commandline.

bwidawsk avatar Jan 02 '24 20:01 bwidawsk

I'd do something like this in your case

#[bpaf(argument::<OutputScales>, env("SBRY_SCALING"), map(|_| warn_the_user()), fallback(()), hide]
pub(crate) scaling: (),

map calls warn_the_user function that prints an error message of your choice as well as replaces parsed value with a stub, fallback makes sure that even if value is absent - parser succeeds and hide removes it from the help message entirely.

warn_the_user can also exit with error so anyone who uses your program knows to update their scripts.

But overall the problem is interesting, I'll think about making it easier to access.

pacak avatar Jan 02 '24 21:01 pacak

But overall the problem is interesting, I'll think about making it easier to access.

Cool. Feel free to close. I'll use your suggestion until something better comes along

bwidawsk avatar Jan 03 '24 18:01 bwidawsk