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.

Sign up for GitHub

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

Jump to bottom

Split generation of help extra items and rendering #2517

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Split generation of help extra items and rendering #2517

wants to merge 1 commit into from

Conversation

kdeldycke
Copy link
Contributor

@kdeldycke kdeldycke commented May 16, 2023 •
edited

This:

This is a small refactor of the original Option.get_help_record() method, splitting it in two to separate the generation of extra items in help screen and their rendering.

This PR:

  • extracts from get_help_record() the code responsible for extra items generation, and move it to its own get_help_extra() method
  • leaves the original get_help_record() as-is, but rely on the new get_help_extra() method to fetch the extra items to render for the help
  • makes the new get_help_extra() returns a dictionary of extra items to be rendered, so get_help_record() can pick them up to render (and translate) them the way it wants
  • augments the original get_help_record() unittests to inspect values returned by get_help_extra()

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.

@kdeldycke kdeldycke mentioned this pull request May 16, 2023
Open
@kdeldycke kdeldycke force-pushed the split-help-extra-generation branch 2 times, most recently from 4bef2d5 to 2b48bdc Compare May 16, 2023 14:30
kdeldycke added a commit to kdeldycke/click-extra that referenced this pull request May 16, 2023
@kdeldycke
Add reference to Click PRs:
6f3d7b8 
pallets/click#2516
pallets/click#2517
@kdeldycke kdeldycke force-pushed the split-help-extra-generation branch 3 times, most recently from 7cd016c to bbe7b72 Compare May 16, 2023 14:50
@kdeldycke
Copy link
Contributor Author

All tests are passing, typing has been fixed and unit-tests have been added. This PR is ready to be merged upstream.

@kdeldycke kdeldycke force-pushed the split-help-extra-generation branch from bbe7b72 to fe1cbbf Compare July 1, 2023 04:55
@davidism davidism added this to the 8.2.0 milestone Jul 4, 2023
@kdeldycke kdeldycke force-pushed the split-help-extra-generation branch 2 times, most recently from 05729eb to 9b80595 Compare July 9, 2023 10:21
@kdeldycke
Copy link
Contributor Author

Thanks @davidism for considering this PR for merging to 8.2.0! I just rebased it following the recent 8.1.4 and it is ready to be reviewed and merged.

@kdeldycke
Copy link
Contributor Author

This PR has been rebased on the latest main branch and all test are passing.

This PR is ready to merged for Click 8.2.0.

@kdeldycke
Split generation of option's help extra and rendering
c3fcf27 
[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
8.2.0
Development

Successfully merging this pull request may close these issues.

Split extra help item computing with help record rendering
2 participants
@kdeldycke @davidism