Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support deprecated options #331

Open
bwidawsk opened this issue Jan 2, 2024 · 4 comments
Open

Support deprecated options #331

bwidawsk opened this issue Jan 2, 2024 · 4 comments

Comments

@bwidawsk
Copy link

bwidawsk commented Jan 2, 2024

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

@pacak
Copy link
Owner

pacak commented Jan 2, 2024

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?

@bwidawsk
Copy link
Author

bwidawsk commented Jan 2, 2024

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.

@pacak
Copy link
Owner

pacak commented Jan 2, 2024

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.

@bwidawsk
Copy link
Author

bwidawsk commented Jan 3, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants