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

Fix panic when using --owner #1164

Merged
merged 2 commits into from
Nov 3, 2022
Merged

Conversation

tmccombs
Copy link
Collaborator

@tmccombs tmccombs commented Nov 3, 2022

Unfortunately, clap_derive can't combine a value_parser of Option with an optional argument to get a merged Option so we need to do the check for the nop outside of the value parser.

Also adds some tests for --owner

Fixes: #1163

@tmccombs tmccombs force-pushed the owner-without-panic branch from ae85cb8 to e4edf87 Compare November 3, 2022 04:43
Unfortunately, clap_derive can't combine a value_parser of Option<T>
with an optional argument to get a merged Option<T> so we need to do the
check for the nop outside of the value parser.

Also adds some tests for --owner

Fixes: sharkdp#1163
@tmccombs tmccombs force-pushed the owner-without-panic branch from e4edf87 to c159ea2 Compare November 3, 2022 05:26
Because macos doesn't have a "root" user
Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Thank you very much!

@sharkdp sharkdp merged commit 99d1db8 into sharkdp:master Nov 3, 2022
@tmccombs tmccombs deleted the owner-without-panic branch November 3, 2022 08:12
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.

Panic when using owner
2 participants