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

Description not included by default #398

Closed
norg opened this issue Nov 2, 2022 · 8 comments · Fixed by #399
Closed

Description not included by default #398

norg opened this issue Nov 2, 2022 · 8 comments · Fixed by #399
Labels
bug Something isn't working component:cli CLI components component:output-formats Supported output formats

Comments

@norg
Copy link

norg commented Nov 2, 2022

Bug description

The readme points out that --desc defaults to auto and in json output mode to on, but when passing --desc auto it does not and if --desc is missing at all the description is also missing. It does work for --desc on and also --desc being passed.

Reproduction steps

Run pip audit with different options and compare json output:

  • pip-audit -r requirements.txt -f json --desc auto -o /tmp/pipaudit_auto.json
  • pip-audit -r requirements.txt -f json --desc on -o /tmp/pipaudit_desc_on.json
  • pip-audit -r requirements.txt -f json --desc -o /tmp/pipaudit_desc.json
  • pip-audit -r requirements.txt -f json -o /tmp/pipaudit.json

Expected behavior

I would have expected to have descriptions always included, the only debate might be if it should be there if --desc is missing overall.

Screenshots and logs

Platform information

  • OS name and version: ArchLinux
  • pip-audit version (pip-audit -V): pip-audit 2.4.5
  • Python version (python -V or python3 -V): Python 3.10.8
  • pip version (pip -V or pip3 -V): ip 22.3 from /usr/lib/python3.10/site-packages/pip (python 3.10)

Additional context

I might be even wrong with my case, so maybe it's just a clarification needed
Thanks for this helpful audit tool!

@norg norg added the bug-candidate Might be a bug. label Nov 2, 2022
@woodruffw
Copy link
Member

Thanks for the report!

Yes, this is intended behavior. To clarify:

  1. --desc auto means "include the description, if it's the default for the selected output format"
  2. --desc on means "always include the description, regardless of the format's default"
  3. --desc off means "never include the description, regardless of the format's default"

Does that clarify things? If so, I can look into tweaking the README to include some of that.

@woodruffw
Copy link
Member

Oh, unless I'm misunderstanding. Are you saying that --desc auto does not produce descriptions in the JSON format?

@norg
Copy link
Author

norg commented Nov 2, 2022

Oh, unless I'm misunderstanding. Are you saying that --desc auto does not produce descriptions in the JSON format?

Yes that's one of the cases where I'm confused about :) Especially with --desc with no additional argument behaving different from the one where auto is added.

@woodruffw
Copy link
Member

Whoops, looks like you're totally right: --desc auto is not behaving correctly. Looks like we're doing a comparison wrong there; sorry for the hasty response!

To clarify, here's the intended behavior of --desc:

  • --desc: same as --desc on
  • --desc on: always emit descriptions, format permitting
  • --desc off: never emit descriptions
  • --desc auto: emit descriptions if the selected format has descriptions by default

@woodruffw
Copy link
Member

I'll have a PR open to fix --desc auto's behavior with the JSON formatter ready in a moment.

@norg
Copy link
Author

norg commented Nov 2, 2022

Thanks for the confirmation. Although I might even argue that --desc should be the same like --desc auto instead of --desc on but at least your list explains why just --desc did work for me :)

@woodruffw woodruffw added bug Something isn't working component:cli CLI components component:output-formats Supported output formats and removed bug-candidate Might be a bug. labels Nov 2, 2022
@woodruffw
Copy link
Member

woodruffw commented Nov 2, 2022

Although I might even argue that --desc should be the same like --desc auto instead of --desc on but at least your list explains why just --desc did work for me :)

I think we did this because --desc on looks a little strange compared to "idiomatic" CLI design: --desc is a boolean option, so its mere presence should imply on rather than needing a value passed (or a non-on state). But I agree it's a little confusing.

@norg
Copy link
Author

norg commented Nov 3, 2022

Awesome, thanks for the explanation and fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:cli CLI components component:output-formats Supported output formats
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants