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

Better defaults for boolean flags #245

Closed
sushantmimani opened this issue Oct 27, 2021 · 6 comments · Fixed by #292
Closed

Better defaults for boolean flags #245

sushantmimani opened this issue Oct 27, 2021 · 6 comments · Fixed by #292
Assignees
@aserputov
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers
Milestone
Version 3.0

Comments

@sushantmimani
Copy link
Contributor

🐛 Bug Report

Boolean flags such as --compact / --no-compact with Default: False is very confusing.

To Reproduce

tartufo -h

Expected Behavior

Should show Default: --no-compact for --compact / --no-compact and similar for other boolean flags
@sushantmimani sushantmimani added bug Something isn't working enhancement New feature or request good first issue Good for newcomers labels Oct 27, 2021
@aserputov
Copy link

@sushantmimani hey! I would like to work on this issue, and if you could provide more information, that would be great. Thanks in advance.

@tarkatronic tarkatronic added this to the Version 3.0 milestone Oct 28, 2021
@sushantmimani
Copy link
Contributor Author

@sushantmimani hey! I would like to work on this issue, and if you could provide more information, that would be great. Thanks in advance.

Hi @aserputov! Sure. Using --compact / --no-compact as an example, when you run tartufo -h and look at all the options you'll currently see something like the below, which is confusing.

--compact / --no-compact        Enable reduced output.  [default: False]

What we want it to look like is:

--compact / --no-compact        Enable reduced output.  [default: --no-compact]

One way to do it would be to change @cli.option(..., show_default=False) to @cli.option(..., show_default="--no-compact") and needs to be done for all boolean options.

@tarkatronic
Copy link
Contributor

Please note that this is targeted at v3.0, so your branch will need to be branched off from the v3.x branch, and then merged back into the same. 😄

@aserputov
Copy link

Sounds good to me. I would like to try it. Thank you.

@sushantmimani
Copy link
Contributor Author

Sounds good to me. I would like to try it. Thank you.

Sounds good! I've assigned this issue to you.

@aserputov aserputov mentioned this issue Oct 29, 2021
Closed
15 tasks
@tarkatronic tarkatronic linked a pull request Nov 4, 2021 that will close this issue
Boolean Flags Update. #246
Closed
15 tasks
@tarkatronic tarkatronic mentioned this issue Dec 7, 2021
Merged
15 tasks
@tarkatronic tarkatronic linked a pull request Dec 7, 2021 that will close this issue
Fix #245: Better boolean flag help output #292
Merged
15 tasks
tarkatronic added a commit that referenced this issue Dec 8, 2021
@tarkatronic
Fix #245: Better boolean flag help output (#292)
d70097e 
* Upgrade click to latest

* Perform some pathing magic in tests to ensure it matches output

* Update CLI help output for docs

* Add a changelog entry about the click upgrade

* Fix PR # for click update

* Another shot at fixing Windows tests

* Match the path resolution to the latest version of click

* Move the output_dir calculation inside the context manager

* Tests become much simpler when you read their output properly.

* Remove the unused import to appease pylint
@tarkatronic
Copy link
Contributor

Fixed in #292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aserputov aserputov

Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
Version 3.0
3 participants
@tarkatronic @sushantmimani @aserputov