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

Introduce Parameter.deprecated + Command.deprecated message customization #2271

Merged
merged 6 commits into from
Nov 29, 2024

Conversation

peacock0803sz
Copy link
Contributor

Implement of #2263, Showing deprecation message with --help for @click.option

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@peacock0803sz peacock0803sz marked this pull request as ready for review May 2, 2022 23:25
@saroad2
Copy link
Collaborator

saroad2 commented May 7, 2023

Hello @peacock0803sz , thanks for the PR and sorry for the long delay.

This seems fine to me, and very helpful for user to use click in a backward compatible way.

However, this PR lacks a very important compnant: Once someone does use a deprecated option, a warning should be printed. This is done today with deprecated commands. You can look here for inspiration.

Please fix this issue before I review this PR again.

@davidism davidism added this to the 8.2.0 milestone Jun 30, 2023
@AndreasBackx AndreasBackx mentioned this pull request Oct 20, 2024
34 tasks
@AndreasBackx AndreasBackx force-pushed the fix/mark-param-deprecated branch 3 times, most recently from 7b559a4 to 38f920f Compare November 2, 2024 17:54
@AndreasBackx AndreasBackx self-assigned this Nov 2, 2024
@AndreasBackx AndreasBackx changed the title Implement to show the message with --help Introduce Parameter.deprecated Nov 2, 2024
@AndreasBackx
Copy link
Collaborator

AndreasBackx commented Nov 2, 2024

I just updated the PR and made the following changes:

  • deprecated has been moved to Parameter meaning an Argument can also be deprecated now. This can be helpful when one wants to perhaps replace an argument with an option.
    • Parameters have an added ! in their usage to indicate that they're deprecated. Anyone know how to make this more clear? I kind of want to make it show up in red, but I'm unsure if that's entirely possible right now without issues.
  • A warning is now shown when the parameter is used, though not when it's a default value.
  • An error is shown (in debug mode) when deprecated=True and required=True. It would be silly to have a deprecated argument be required. How does one otherwise move away from it?
    • Also when deprecated=True and Option.prompt is truthy.
  • Tests to cover these use cases.

I noticed that Option's constructor raises TypeErrors for bad values. Shouldn't those be ValueError?

I'm thinking it's a good idea to allow passing a str instead of solely a boolean to give the user some context. (Added)

@ThiefMaster
Copy link
Member

What's the reason for disallowing prompt w/ deprecated args?

@AndreasBackx
Copy link
Collaborator

@ThiefMaster as with required=True. When setting deprecated=True, there needs to be an avenue for the user to move away from it. How can they move away when they will be automatically prompted for it?

Correct me if my understanding around the use of prompt is incorrect. I am not a frequent user of it.

@AndreasBackx
Copy link
Collaborator

AndreasBackx commented Nov 2, 2024

Follow-up question: should Command.deprecated not also allow a string to be passed so a custom message can be printed?

Done

@ThiefMaster
Copy link
Member

Ah yes, I thought it prompts for a value when specifying the option. But it prompts when the option is not provided so I guess it makes more sense to disallow it.

@AndreasBackx AndreasBackx added the f:parameters feature: input parameter types label Nov 2, 2024
@AndreasBackx AndreasBackx changed the title Introduce Parameter.deprecated Introduce Parameter.deprecated + Command.deprecated message customization Nov 2, 2024
@AndreasBackx AndreasBackx requested a review from davidism November 9, 2024 19:03
@AndreasBackx
Copy link
Collaborator

There seem to be no comments so I am merging this.

@AndreasBackx AndreasBackx force-pushed the fix/mark-param-deprecated branch from 2daf826 to 66d36d5 Compare November 29, 2024 00:21
@AndreasBackx AndreasBackx merged commit 5961d31 into pallets:main Nov 29, 2024
12 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f:parameters feature: input parameter types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mark parameter as deprecated
5 participants