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

First stab at nicely formatted cli help output using Rich. #1403

Merged
merged 6 commits into from
Feb 9, 2022

Conversation

ewels
Copy link
Member

@ewels ewels commented Feb 9, 2022

Took code from rich-cli written by @willmcgugan and played around with until it worked for us.

It's so pretty 🤩

  • Help strings are no longer truncated if they're a bit on the long side
  • Text now stuff wraps onto multiple lines for narrow terminals and long help texts
  • I've set it to use a maximum width of 100 chars, instead of the full terminal width. Personal preference.
  • We have a custom @click.version_option() which doesn't seem to make it into the help, I don't know how to fix this.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated
Before Afer
image image
image image

Copy link
Member

@maxulysse maxulysse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

@ewels ewels added the WIP Work in progress label Feb 9, 2022
@ewels ewels marked this pull request as draft February 9, 2022 15:41
@ewels
Copy link
Member Author

ewels commented Feb 9, 2022

Don't merge yet - just figured out why the version wasn't printed. We're skipping the first argument because I copied over some code without understanding why it was there 👀

The rich-cli has a positional argument which I guess looked a bit ugly in the help, so was skipped. We also often have positional arguments but not always (in which case the first option flag is skipped), and in the future some commands may have more than one (in which case subsequent ones will be printed).

Now looking into a way to detect these and give them dedicated styling.

@ewels ewels marked this pull request as ready for review February 9, 2022 15:59
@ewels ewels removed the WIP Work in progress label Feb 9, 2022
@ewels
Copy link
Member Author

ewels commented Feb 9, 2022

Ok, sorted it. I started adding support for a panel dedicated to arguments, but then realised that there isn't really much that we can print. click doesn't accept help strings for arguments and there are no options, so it's basically only the metavars. These are already printed in the usage string. So now the code just skips any parameters that are positional arguments.

It seems that this is intentional on the part of click. See relevant docs: https://click.palletsprojects.com/en/8.0.x/documentation/#documenting-arguments

@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #1403 (2351867) into dev (0ddcd23) will increase coverage by 0.10%.
The diff coverage is 85.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1403      +/-   ##
==========================================
+ Coverage   66.78%   66.88%   +0.10%     
==========================================
  Files          50       50              
  Lines        5714     5762      +48     
==========================================
+ Hits         3816     3854      +38     
- Misses       1898     1908      +10     
Impacted Files Coverage Δ
nf_core/__main__.py 58.07% <85.33%> (+2.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 224c24d...2351867. Read the comment docs.

@ewels ewels merged commit d30f27b into nf-core:dev Feb 9, 2022
@ewels ewels deleted the rich-help branch February 9, 2022 18:04
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

Successfully merging this pull request may close these issues.

2 participants